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

Allow custom ordering of script phases (#204) #210

Closed
wants to merge 4 commits into from

Conversation

champo
Copy link

@champo champo commented Jan 22, 2015

Extended the script phase configuration to include a new form that
accepts a key index. The value is then used as index for inserting the
phase into the project.

Extended the script phase configuration to include a new form that
accepts a key index. The value is then used as index for inserting the
phase into the project.
target.build_phases.delete(build_phase)

# Sanitize values so that no exception come about
if index >= target.build_phases.length or index < -1

Choose a reason for hiding this comment

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

Use || instead of or.

puts "Adding shell script build phase '#{value}'"
add_shell_script_build_phase(file_manager.template_contents(key), value)

# -1 means last as index
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding this comment, maybe make a constant to hold the value: LAST_INDEX = -1

@gfontenot
Copy link
Member

Neato. I think that we should change the format over entirely. I always felt a little weird about abusing dictionaries like we were doing anyway. This way, you could just have an optional index key if you care about it.

After those fixes, could you remove the code comments? I don't think they are needed.

@champo
Copy link
Author

champo commented Feb 4, 2015

yeah, I did backwards compatible because I didn't think I was in a place to
decide whether breaking it was OK. I'll update these in as soon as I can.

On Wed, Feb 4, 2015, 14:35 Gordon Fontenot notifications@github.com wrote:

Neato. I think that we should change the format over entirely. I always
felt a little weird about abusing dictionaries like we were doing anyway.
This way, you could just have an optional index key if you care about it.

After those fixes, could you remove the code comments? I don't think they
are needed.


Reply to this email directly or view it on GitHub
#210 (comment).

@gfontenot
Copy link
Member

I appreciate that. Thanks. I think we can break backwards compatibility a bit for the sake of flexibility and maintainability.

@@ -1,5 +1,7 @@
module Liftoff
class XcodeprojHelper

LAST_INDEX = -1
Copy link
Member

Choose a reason for hiding this comment

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

Remove the whitespace line above this and add a whitespace line after it.

@gfontenot
Copy link
Member

One tiny style comment (that I can fix if you'd like) then this is ready to go.

@gfontenot
Copy link
Member

Awesome. Thanks so much for this. Squashed it down and reworded the man page a bit, but this got merged in 98814ec

@gfontenot gfontenot closed this Feb 9, 2015
@bitcrumb
Copy link
Contributor

bitcrumb commented Feb 9, 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