Several Small Changes #44

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

ddrscott commented Oct 2, 2012

  • allow vender.headers_dir to be an array
  • added back App.configs for BubbleWrap dependency
  • fixed order of operations in option parsing
  • ignore copy of hidden directories (example: .svn) in resource copy.
  • add configuration_build_dir to vender setting

I was originally planning on giving each change as a separate branch, but it because too hard for me to maintain and due to other things that would break in my specific huge vendor project and a Cocao Pods dependency, I couldn't validate my build worked unless ALL these changes where in.
Trying to tease them out into separate branches would have burnt away too much time.

Let me know if you really really really need them separately and I see what I can do.

Look at the commit comments to figure out the changes.

This also include the patch-1 I submitted a couple days ago, so you can ignore that one.

@lrz lrz commented on the diff Oct 8, 2012

lib/motion/project/app.rb
@@ -56,6 +58,14 @@ def config_without_setup
@configs[config_mode] ||= Motion::Project::Config.new('.', config_mode)
end
+ def configs
+ @configs ||= {}
+ MODES.each do |mode|
+ @configs[mode] ||= Motion::Project::Config.new('.', mode)
+ end
+ @configs
+ end
+
@lrz

lrz Oct 8, 2012

Owner

What's the rationale behind this change? The problem as I see it is that 2 Config objects will be created and I don't see the point in exposing this new method, since we already have the app.development and app.release methods to isolate development/release settings in the Rakefile.

@ddrscott

ddrscott Oct 8, 2012

Contributor

The Pods project is still calling App.configs and was crashing during vendor build with a No Method Error.
I think they (the Pods) project needed a deprecation warning or something before removing the public method.
I tried to keep the nature of the old #configs method behavior. I actually don't care how it is implemented, as long as the Pods project works :)

@lrz

lrz Oct 8, 2012

Owner

Oh I see, I remember, that was an internal method that BubbleWrap depended upon. We sent a pull request and I believe it has been merged, so there is probably no need to add this method back.

@lrz lrz commented on the diff Oct 8, 2012

lib/motion/project/vendor.rb
@@ -184,9 +184,13 @@ def build_xcode(platform, opts)
end
end
+ def configuration_build_dir
+ File.absolute_path(@opts[:configuration_build_dir] || 'build')
+ end
+
@lrz

lrz Oct 8, 2012

Owner

I believe that c66f4fa is a better solution, as it's simpler and does not introduce a new option. Using ".build" is probably safe enough.

@ddrscott

ddrscott Oct 8, 2012

Contributor

I'll respectfully disagree simply because the patch build value is
hardcoded. I'd rather a configurable option default to '.build' than have
some one else hunt for like I did :-
On Oct 8, 2012 6:06 PM, "Laurent Sansonetti" notifications@github.com
wrote:

In lib/motion/project/vendor.rb:

@@ -184,9 +184,13 @@ def build_xcode(platform, opts)
end
end

  • def configuration_build_dir
  •  File.absolute_path(@opts[:configuration_build_dir] || 'build')
    
  • end

I believe that c66f4fahttps://github.com/HipByte/RubyMotion/commit/c66f4faeff42db3d27b1a21f5ed68a18e1fa7a85is a better solution, as it's simpler and does not introduce a new option.
Using ".build" is probably safe enough.


Reply to this email directly or view it on GitHubhttps://github.com/HipByte/RubyMotion/pull/44/files#r1790157.

colinta closed this Jul 31, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment