-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update version comments for SHA-pinned GitHub Actions #5951
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| on: [push] | ||
|
|
||
| name: Integration | ||
| jobs: | ||
| chore: | ||
| steps: | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # 2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # @v2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # pin @v2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # tag=v2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0 | ||
| # The comment on the next line has a trailing tab. The version should still be updated. | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0 | ||
| - uses: actions/checkout@01aecc # v2.1.0 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Major 💖 for phenom set of test cases. Two more cases:
I don't want to throw up roadblocks on this awesome PR, so if for simplicity neither case is bumped, that's perfectly acceptable. So one simple heuristic could be if there's any text after the version string, then don't bump the version string... Users that still want it bumped can instead leave their comment as a full-line comment above the actual code line: But we do need to ensure we never auto-bump the comment to start incorrectly saying So can you add these two test cases to ensure no future regressions?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the feedback. I was worried the simple search-and-replace was too broad but I couldn't come up with a sample comment where we wouldn't want to bump the version. The cases you mentioned are exactly what I was looking for! I've updated the PR with the simple fix to only change comments if there's no text after the version string. |
||
| integration: | ||
| - uses: actions/checkout@v2.1.0 # comments that include the version (v2.1.0) shouldn't be updated for non-SHA refs | ||
| - uses: actions/checkout@01aecc#v2.1.0 # this shouldn't be updated, because the version is part of the ref, not a comment. | ||
|
|
||
| # The version in the comment for the next action shouldn't be updated | ||
| # because it refers to past behavior. | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # Versions older than v2.1.0 have a security vulnerability | ||
|
|
||
| # The versions in the comment for the next action won't be updated. | ||
| # The first version could be updated, but it's difficult to create | ||
| # a heuristic that recognizes the first version as a version alias | ||
| # for the SHA commit, and the second version as a concrete version | ||
| # that shouldn't change. For simplicity, we don't update either. | ||
| - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 - Versions older than v2.1.0 have a security vulnerability | ||
Uh oh!
There was an error while loading. Please reload this page.