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.

Sign up for GitHub

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

Fix default branch reference in GitHub Actions workflow definitions #278

Merged
merged 4 commits into from
Oct 5, 2022
Merged

Fix default branch reference in GitHub Actions workflow definitions #278

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbf0e77
Revert default branch
japborst Oct 5, 2022
c779c09
Use single line
japborst Oct 5, 2022
884e108
Add spaces for consistency
japborst Oct 5, 2022
15e117c
Hardcode master
japborst Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ name: Build and verify
on:
pull_request:
push:
branches: [$default-branch]
branches:
- master
Copy link
Member

Choose a reason for hiding this comment

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

We could still do:

Suggested change
branches:
- master
branches: [ master ]

(Assuming the earlier intention was to match jdk: [ 11.0.16, 17.0.4 ] below. Or we could change both to multi-line lists. Maybe that's nicer?)

Copy link
Member

Choose a reason for hiding this comment

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

IMO the jdk field is a bit different as for matrix that is the way to define the values, see here.

AFAICS that is the only way there. For branches, especially if it is only one, I prefer the - master way.

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

The docs also use both approaches, not very consistent. [something] is the same as - something, so in the end shouldn't matter. It can be a shorthand, which are especially helpful for matrices, whereas branches are longer to begin with.

jdk: [ 11.0.16, 17.0.4 ]

instead of

jdk:
 - 11.0.16
 - 17.0.4

Nevertheless, I don't see us expanding this list to a wide range, so using the shorthand is pretty clean.

permissions:
contents: read
jobs:
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/deploy-website.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ name: Update `error-prone.picnic.tech` website content
on:
pull_request:
push:
branches: [$default-branch]
branches:
- master
permissions:
contents: read
id-token: write
Expand Down