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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the correct SHA for dependency snapshots from pull requests #401

Merged
merged 1 commit into from Mar 15, 2023

Conversation

juxtin
Copy link
Contributor

@juxtin juxtin commented Mar 15, 2023

馃憢馃徑 from the Dependency Graph team at GitHub!

I've been working on some functionality related to dependency snapshots on pull requests, and I've discovered an unfortunate fact about the behavior of github.context.sha/$GITHUB_SHA in that context.

Evidently, github.context.sha is not always what you might consider the "current commit" in a given context. In most GitHub pull request event types, github.context.sha will be the "Last merge commit on the GITHUB_REF branch" (See Actions documentation here). In those cases, the commit SHA that should be associated with the snapshot is github.event.pull_request.head.sha.

This PR adds a helper function, getSha, which will return the correct SHA for the current context, and uses that function to find the right SHA to add to the dependency snapshot.

Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

LGTM -- these are some of the hardest things to test: integration between tools! I'd ask for a test, but I feel it would be so synthetic that it would be meaningless. Thanks a lot for the contribution!

@kzantow
Copy link
Contributor

kzantow commented Mar 15, 2023

@juxtin could you sign off the commits, though, please?

@juxtin
Copy link
Contributor Author

juxtin commented Mar 15, 2023

Thanks @kzantow! I believe the commit is already signed off, but I admit I'm not used to doing that so perhaps I did it incorrectly?

Evidently, `github.context.sha` is not always what you might consider
the "current commit" in a given context. In most GitHub pull request
event types, `github.context.sha` will be the "Last merge commit on the
GITHUB_REF branch." In those cases, the commit SHA that should be
associated with the snapshot is `github.event.pull_request.head.sha`.

This commit adds a helper function, `getSha`, which will return the
correct SHA for the current context.

Signed-off-by: Justin Holguin <juxtin@github.com>
Signed-off-by: GitHub <noreply@github.com>
@juxtin
Copy link
Contributor Author

juxtin commented Mar 15, 2023

I tried again, for some reason it expected it to be signed off by GitHub as well as by me personally.

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.

None yet

2 participants