Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added 'pre' and 'post' setup blocks #174

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Added 'pre' and 'post' setup blocks #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c6db30
added 'pre' and 'post' setup blocks
colinta Oct 14, 2014
ae4579e
oh shoot, I forgot to copy in this refactor
colinta Oct 15, 2014
936b9fe
using 'pre_setup_blocks' we can set 'app.files' before the Rakefile s…
colinta Oct 15, 2014
176f291
this was calling 'setup' twice, since 'config' also calls 'setup'
colinta Oct 15, 2014
7f766b8
Merge branch 'master' into pre_post_setup_blocks
colinta Jan 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/motion/project/app.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,16 @@ def builder
end

def setup(&block)
config.setup_blocks << block
config_without_setup.setup_blocks << block
config
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling config here is what causes some troubles, because config ends up determining sdk and api versions. For instance, on android, I need to set app.api_version = '19', otherwise I get an error about the "L" SDK not being installed.

Which is fine until I include a gem, which calls setup, which calls config, and goes looking for the api_version. My Rakefile's setup hasn't been called, so it detects the wrong one, and errors out.


def pre_setup(&block)
config_without_setup.pre_setup_blocks << block
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre_setup uses the same @setup_blocks as setup, but doesn't call config.setup at the end.


def post_setup(&block)
config_without_setup.post_setup_blocks << block
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post_setup on the other hand uses a new ivar, since the Rakefile setup needs to be called in between the pre and post setup blocks.

Ok, so all that said, it wouldn't be any trouble to use a more explicit @pre_setup = [] array of blocks. There's no advantage other than being explicit, though.

end

def build(platform, opts={})
Expand Down
38 changes: 37 additions & 1 deletion lib/motion/project/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ def self.make(template, project_dir, build_mode)

def initialize(project_dir, build_mode)
@project_dir = project_dir
@files = Dir.glob(File.join(project_dir, 'app/**/*.rb'))
@files = []
@app_dir = 'app'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, to me, the most controversial change. Previously in the setup block, the app.files has already been set, but this delays that until after all setup blocks are called.

The reason for the change is so that gems, using pre_setup, can append their files to app.files. Right now they have to be inserted before the app files, otherwise dependencies get very messy... Ideally, to keep things identical, app.files would be set before the Rakefile's setup block is called.

Oh, shoot, I just realized that using @pre_setup blocks would allow for this... OK one sec I'm gonna do that change.

@build_mode = build_mode
@name = 'Untitled'
@resources_dirs = [File.join(project_dir, 'resources')]
Expand Down Expand Up @@ -106,16 +107,51 @@ def variables
map
end

def pre_setup_blocks
@pre_setup_blocks ||= []
end

def setup_blocks
@setup_blocks ||= []
end

def post_setup_blocks
@post_setup_blocks ||= []
end

def setup
should_validate = false

if @pre_setup_blocks
@pre_setup_blocks.each { |b| b.call(self) }
should_validate = true
@pre_setup_blocks = nil
end

if @app_dir
Dir.glob(File.join(project_dir, "#{@app_dir}/**/*.rb")).each do |app_file|
@files << app_file unless @files.include?(app_file)
end
should_validate = true
@app_dir = nil
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, ignore the "controversial" stuff above, this restores the previous behavior, where app.files is set before the Rakefile setup block is called.


if @setup_blocks
@setup_blocks.each { |b| b.call(self) }
should_validate = true
@setup_blocks = nil
end

if @post_setup_blocks
@post_setup_blocks.each { |b| b.call(self) }
should_validate = true
@post_setup_blocks = nil
end

if should_validate
validate
end

self
end

Expand Down