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

Extend script phase DSL to support execution position #414

Merged

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Oct 4, 2017

@paulb777 I won't get into the habit of adding you as a reviewer. First of all thanks for your time on these and second it's only because you've reviewed the other PRs.

@dnkoutso dnkoutso requested a review from paulb777 October 4, 2017 20:34
@dnkoutso dnkoutso added this to the 1.4 milestone Oct 4, 2017
@@ -368,6 +368,10 @@ def target(name, options = nil)
# @option options [Boolean] :show_env_vars_in_log
# whether this script phase should output the environment variables during execution.
#
# @option options [Symbol] :execution_placement
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you like :execution_placement? Would you prefer another word? I am not super happy but i cant find a better word.

Copy link
Member

Choose a reason for hiding this comment

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

position ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've thought about it and re-thought about it again. I like it. Let's go with this instead.

@dnkoutso dnkoutso force-pushed the script_phase_execution_placement_dsl branch from 3551b64 to 1550382 Compare October 4, 2017 20:58
@dnkoutso dnkoutso changed the title Extend script phase DSL to support execution placement Extend script phase DSL to support execution position Oct 4, 2017
@dnkoutso
Copy link
Contributor Author

dnkoutso commented Oct 4, 2017

Updated to use position everywhere instead of placement.

@dnkoutso dnkoutso force-pushed the script_phase_execution_placement_dsl branch 2 times, most recently from 6278f3d to ff97759 Compare October 4, 2017 21:03
@parent.script_phases.should == [
{ :name => 'PhaseName', :script => 'echo "Hello World"', :shell_path => :'/usr/bin/ruby' },
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 wrong :'/usr/bin/ruby' with the : added. Fixed.

@dnkoutso dnkoutso force-pushed the script_phase_execution_placement_dsl branch from ff97759 to d6518be Compare October 4, 2017 21:23
@dnkoutso
Copy link
Contributor Author

dnkoutso commented Oct 4, 2017

All green.

@dnkoutso dnkoutso force-pushed the script_phase_execution_placement_dsl branch 3 times, most recently from 0b8b9c1 to fd031ca Compare October 4, 2017 21:43
@dnkoutso dnkoutso force-pushed the script_phase_execution_placement_dsl branch from fd031ca to 4ef270f Compare October 4, 2017 22:20
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.

3 participants