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

Adding support to creating custom build_configurations #224

Closed
wants to merge 1 commit into from
Closed

Adding support to creating custom build_configurations #224

wants to merge 1 commit into from

Conversation

marcelofabri
Copy link
Contributor

This enables creation of custom build configurations by adding build_configurations to .liftoffrc:

build_configurations:
    - name: Debug-CI
      type: debug
    - name: Release-CI
      type: release

type must be release or debug (or else it'll error in add_build_configuration). As xcodeproj itself doesn't explicitly checks for these, I followed the same rule. I also haven't checked if both name and type were specified as I haven't seen this pattern through the codebase.

create_build_configurations was called before configure_base_project_settings so all further configuration is also applied to the created configurations.

The created build configurations should be accessible from extra_config, but I haven't tested it yet.

UPDATE: I have tested using extra_config and it did work as expected.

Also, I haven't updated the documentation yet and haven't created tests for this (some help here would be greatly appreciated).

@bitcrumb
Copy link
Contributor

Good stuff 👍🏻

@solrankos
Copy link

This would help me so much! 👍 🔝

builder = BuildConfigurationBuilder.new(xcode_project)
build_configurations ||= []

build_configurations.each do |configuration|
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about providing a method on the BuildConfigurationBuilder that accepts an array of build configurations and adds them to the project? Pretty much copy and pasting this section into there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was my first thought, but then I thought that it'd be better if BuildConfigurationBuilder didn't have to deal with any hashes (to avoid an informal contract about the keys). Does that make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. I don't feel super strong either way. To play devil's advocate:

Could we safely assume that the generate_build_configurations method accepts an array of hashes with a name and type and in this case they happen to match? If they keys ever change we could update this section to pass in that structure and not have to change the class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've added a generate_build_configurations method.

@jakecraige
Copy link
Contributor

Looks great! Is this still a WIP? Feel free to squash down when it's ready

@marcelofabri
Copy link
Contributor Author

Yeah, I still need to add some documentation than we're ready!

@jakecraige
Copy link
Contributor

Good catch :) Feel free to ping me when you're ready and I'll take a look.

@marcelofabri
Copy link
Contributor Author

@jakecraige Done!

@jakecraige
Copy link
Contributor

Good stuff. Merged in ce47d4b

@jakecraige jakecraige closed this Sep 20, 2015
@marcelofabri
Copy link
Contributor Author

🎉

@marcelofabri marcelofabri changed the title [WIP] Adding support to creating custom build_configurations Adding support to creating custom build_configurations Oct 6, 2015
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.

4 participants