-
Notifications
You must be signed in to change notification settings - Fork 61
Refer to actions by commit hash #196
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
base: main
Are you sure you want to change the base?
Conversation
bdb2f2f
to
cc6a179
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in such an organization! ..and I've also been thinking about getting this into sbt-github-actions
.
Regarding churn I'm not convinced it'd be a huge problem. Updates isn't currently automated by dependabot anyway, so the need to update dependencies by issuing PRs isn't really changed. But sure, it puts a strain on the reviewer who has to verify that the hash matches the intended version.
|
||
object UseRef { | ||
final case class Public(owner: String, repo: String, ref: String) extends UseRef | ||
final case class Public(owner: String, repo: String, rev: String, ref: String) extends UseRef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing as ref
is now always rendered as a comment, I think it makes to keep ref
as the actual reference used (be it a hash, tag, or branch name), and instead introduce comment: Option[String] = None
. This way we'd retain source compatibility, also allowing users to opt-in to reference actions by a hash + comment, if they aren't already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure - I think it's actually a feature that users would have to review the call site to make sure the third argument is a rev
. I could see making the fourth parameter a comment: Option[String]
though, no strong preference.
2453fa6
to
2f543bf
Compare
Some organizations have a policy of always referring to GitHub actions by commit hash rather than tag, as tags can be moved without review. This explores what this would look like for `sbt-github-actions`. Of course the main downside would be that updating the hashes creates churn.
2f543bf
to
77948c4
Compare
UseRef.Public("actions", "checkout", "v5"), | ||
UseRef.Public( | ||
"actions", | ||
"checkout", | ||
"08c6903cd8c0fde910a37f88322edcfb5dd907a8", | ||
"v5.0.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sympathetic to having an immutable and repeatable CI configuration, but the downside to embedding git sha into *.scala
code this way is that now the upgrade burden would shift to the sbt-github-actions maintainer and/or simply this would be left forgotten as the actions might ship fixes and patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jup - this PR was more of a 'straw man' to get the conversation going than something I'd expect to get merged in this form.
What would be the way forward here? Perhaps we could have an option that makes githubWorkflowCheck
more lenient and allow dependabot to update versions (but not the rest of the workflow)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the limited number of upstream actions, we could probably run some weekly cron job that check them out and maintain a JSON file of action, version, and gitsha, maybe?
Some organizations have a policy of always referring to GitHub actions by commit hash rather than tag, as tags can be moved without review.
This explores what this would look like for
sbt-github-actions
. Of course the main downside would be that updating the hashes creates churn.