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

Get the right SHA for the snapshot depending on the event type #42

Merged
merged 2 commits into from
Mar 16, 2023

Conversation

juxtin
Copy link
Contributor

@juxtin juxtin commented Mar 16, 2023

As noted in the Actions docs, context.sha can represent different things in different event types. In a pull_request event, it happens not to be the sha of the head commit for some reason. Instead, in those cases you're meant to get the head sha from the pull_request payload.

This PR introduces a helper function, shaFromContext, that matches on the event type and gets the correct sha for the snapshot. I've also taken the liberty of doing a few quick test refactors.

For prior art, see:

@juxtin juxtin requested a review from a team as a code owner March 16, 2023 16:45
@juxtin juxtin merged commit cfc29cb into main Mar 16, 2023
@juxtin juxtin deleted the juxtin/sha-from-context branch March 16, 2023 17:16
* See https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request for more details
* about why this function is necessary, but the short reason is that GITHUB_SHA is _not_ necessarily the head sha
* of the PR when the event is pull_request (or some other related event types).
*

Choose a reason for hiding this comment

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

☝️ This is such a gotcha for folks building DG snapshot Actions I'm glad to see us call it out in the toolkit code here! ❤️

🤔 I wonder what else we could do to document the "so you're building a DG snapshot Action" gotchas somewhere public for folks that don't choose to use the toolkit under the hood? Maybe a blog post about the quirk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about sneaking something about this into the API docs, since I think that's all we really have now!

MuhittinTekin

This comment was marked as spam.

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

4 participants