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.

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

[Project] Create all configurations for new_target #229

Merged
merged 1 commit into from
Jan 24, 2015

Conversation

neonichu
Copy link
Member

The new_target method now creates build configurations corresponding to all configurations of the project, not just Debug and Release.

Fixes #228

@@ -157,6 +157,15 @@ def self.configuration_list(project, platform, deployment_target = nil, target_p

cl.build_configurations << release_conf
cl.build_configurations << debug_conf

project.build_configurations.each do |configuration|
next if %w(Debug Release).include?(configuration.name)
Copy link
Member

Choose a reason for hiding this comment

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

next if cl.build_configurations.map(&:name).include?(configuration.name) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even better:

cl.build_configurations  = project.build_configurations.reject do |configuration|
  ...
end.map do |configuration|
  new_config = project.new(XCBuildConfiguration)
  new_config.name = configuration.name
  new_config
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Scratch that actual example, didn't read the above yet (needs to factor in release_conf and debug_conf)

Copy link
Contributor

Choose a reason for hiding this comment

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

So, take #2 (because I really hate array.append when map will suffice):

cl.build_configurations = project.build_configurations.map do |configuration|
  config = project.new(XCBuildConfiguration)
  config.name = configuration.name
  if config is debug or release
    config.build_settings = common_build_settings(configuration.name.downcase.to_sym, platform, deployment_target, target_product_type)
  end
  config
end

This removes 8+ so lines above too 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like build configurations can only be added one-by-one, though, to the XCConfigurationList- probably deliberately so.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also seems like the presence and contents of the default configurations isn't well-tested enough to really touch :) The first parameter of common_build_settings has significance to what gets added, so needs to be :debug or :release to not change behaviour.

@segiddins
Copy link
Member

@neonichu 👍 once my comments are addressed

The `new_target` method now creates build configurations
corresponding to all configurations of the project, not just Debug
and Release.
@neonichu neonichu force-pushed the new-target-all-the-build-configurations branch from 95c19ad to 0e672e0 Compare January 21, 2015 09:57
kylef added a commit that referenced this pull request Jan 24, 2015
…gurations

[Project] Create all configurations for new_target
@kylef kylef merged commit bbfe89e into master Jan 24, 2015
@kylef kylef deleted the new-target-all-the-build-configurations branch January 24, 2015 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

project.new_target does only adds "Release" and "Debug" configurations
3 participants