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

Reference actions by commit SHA #833

Merged
merged 1 commit into from May 30, 2023
Merged

Reference actions by commit SHA #833

merged 1 commit into from May 30, 2023

Conversation

gabibguti
Copy link
Contributor

Resolves #832

It's important to make sure the SHA's are from the original repositories and not forks. I have manually verified them and added references in the commit description.

It's important to make sure the SHA's are from the original repositories and not forks.

For reference:

https://github.com/actions/checkout/releases/tag/v3.5.2
actions/checkout@8e5e7e5

https://github.com/msys2/setup-msys2/releases/tag/v2
msys2/setup-msys2@7efe20b

Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com>
# | Item | Section in the right pane |
# | ------------------------- | ---------------------------------------------------------------------- |
# | OS, VM | Set up job |
# | git repo, commit hash | Run actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2 |
Copy link
Owner

Choose a reason for hiding this comment

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

Is it useful to also update the comment ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Up to you! We can revert to Run actions/checkout@v3 or change to Run actions/checkout@v3.5.2, whatever feels easier to understand here.

Copy link
Owner

@Cyan4973 Cyan4973 May 13, 2023

Choose a reason for hiding this comment

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

Indeed, it's just an overview,
I would assume Run actions/checkout@v3 to be significant enough here.

I'm just concerned about some potential future automated code changes, like possibly dependabot, that could miss the comment nature of this line.

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 have tested renovatebot on this case. The bot updates only the real action line and not the table comment line. So, the table comment and the action may get unsynced over time.

Copy link
Contributor Author

@gabibguti gabibguti May 29, 2023

Choose a reason for hiding this comment

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

Complementing, I have tested comment lines such as:

  • # | Run actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.2 |
  • # actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab
  • # actions/checkout@v3

and none get updated by renovatebot.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that's why I was more in favor of keeping it simple and generic, since it won't get updated with the rest of the code.
But don't worry, I'll do it directly.

@Cyan4973 Cyan4973 merged commit 5562a14 into Cyan4973:dev May 30, 2023
56 checks passed
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.

Reference actions by commit SHA
2 participants