-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
base: master
Are you sure you want to change the base?
Conversation
Also, since |
end | ||
|
||
# should we include a check here for "already included app/ files"? it's | ||
# not necessary since this operation is idempotent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m unclear as to why not to check it, especially since you do it after all :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh shoot, I just realized I refactored this in /Library/RubyMotion, and didn't copy that in. I'm gonna push that up (with comments to hopefully answer some of these questions)
Can you elaborate on this? Why should motion-cocoapods use that hook instead? E.g. what config access should it be using? |
I didn't mean Motion::Project::App.pre_setup do |app|
# add my gem files to app.files
end
Motion::Project::App.post_setup do |app|
installed_pods = app.pods
# I just went looking for a method that could return true/false for "did they include this cocoapod", but
# couldn't find one... but if it DID exist, this is what I was imagining:
unless installed_pods.include?('AFNetworking')
raise StandardError, 'The AFNetworking library is required for this gem, please add it to your cocoapods'
end
end Also, code analyzing tools like dbt are ideal for using the Motion::Project::App.post_setup do |app|
DBT.analyze(app)
end |
config.setup_blocks << block | ||
config_without_setup.setup_blocks << block | ||
config.setup | ||
end |
There was a problem hiding this comment.
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.
…etup block is called, just like old times
end | ||
should_validate = true | ||
@app_dir = nil | ||
end |
There was a problem hiding this comment.
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.
Now that I think more on it, the most important features of
|
Btw, @alloy, when I was looking through cocoapods code, I noticed this line: https://github.com/HipByte/motion-cocoapods/blob/master/lib/motion/project/cocoapods.rb#L74 This assumes |
@colinta Do you have an example gem that has been updated to use this so we can easily test and verify it? |
Yes, the android branch of sugarcube. It also has a "support" file that adds pre/post setup methods if they're not defined.
|
* master: (157 commits) 3.2 use quoted string instead of #shellescape [iOS] When stripping symbols, ignore symbols from the symbols file that are not present in the executable. revert deleting i386 for simulator Fixes the extra-packages arg to work with multiple packages [iOS] Silence `strip` in non-verbose mode. introduce :native parameter for app.vendor_project to include 3rd-party shared libraries [iOS] Do not strip symbols from host that a framework requires. [RM-696] escape Watch app's Entitlements.plist path because it contains white spaces. [RM-695] add 'application-identifier' to entitlements in order to deploy app to iOS 8.2 beta 3 device remove `i386' at :build:simulator task remove 'i386' only if build 'x86_64' reduce compile time by removing `i386' for iOS simulator [iOS] Pick-up new certificates prefixed with `iOS` instead of `iPhone`. [WatchKit] Honor `xcode_dir` setting from host app. [log] Show backtrace on `App.fail` and in verbose mode. 3.1 [iOS Ext] Don't use private API, try to replicate Xcode closely. [iOS Ext] Always pass on the full env to the target. [iOS Ext] Fix startup crashes by reverting to previous behaviour. ...
I would love to have these methods. 👍 |
@files
is just an empty array at this pointsdk_version
and so on can still be set by the user