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

Use github.head_ref as branch default value #75

Merged
merged 1 commit into from
May 10, 2020

Conversation

stefanzweifel
Copy link
Owner

${{ github.head_ref }} is only filled when a workflow listens to the pull_request event. However, it evaluates to an empty string when listening to other events (the same default value as the current version has).

By doing this change, the branch-input no longer must manually added to a workflow when listening to pull_request. But, the actions/checkout step still has to be updated and ref has to be added. Otherwise, the repo would be cloned in a detached head and this Action can't commit and push correctly

    - uses: actions/checkout@v2
      with:
        ref: ${{ github.head_ref }}

refs #73


I'm not sure yet, if this is the right approach.
Actions are basically just configuration code. I think being more explicit in that configuration code leads to less confusion and easier to understand workflows.

In addition, the user still needs to update the checkout-step. So a consumer can't just add 2-3 lines of code to add one single step. They have to update the rest of their workflow too. 🤔

@spawnia
Copy link
Contributor

spawnia commented May 10, 2020

I can not think of a scenario where the default '' would be better.

Sane defaults generally make things easier to use and are also a great way to smooth over potential upgrades or breakages. It saves every user the trouble of considering possible configuration values.

@stefanzweifel
Copy link
Owner Author

Yeah, I think this is the way to go.

@stefanzweifel stefanzweifel merged commit fd2aab7 into master May 10, 2020
@stefanzweifel stefanzweifel deleted the use-head-ref-as-default-value branch May 10, 2020 10:31
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.

2 participants