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

Add SHA property #192

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Add SHA property #192

merged 1 commit into from
Aug 13, 2021

Conversation

flcdrg
Copy link
Contributor

@flcdrg flcdrg commented Aug 10, 2021

Implements #191

@flcdrg
Copy link
Contributor Author

flcdrg commented Aug 11, 2021

@NasAmin is there an issue with the unit tests? I see other PRs are also failing with a similar error.

@NasAmin
Copy link
Owner

NasAmin commented Aug 12, 2021

@NasAmin is there an issue with the unit tests? I see other PRs are also failing with a similar error.

Hi @flcdrg Thanks for the PR! I'll look into the issue this evening (UK time).

@flcdrg
Copy link
Contributor Author

flcdrg commented Aug 12, 2021

No worries. I've regenerated the dist files from within WSL (previously I'd committed on Windows), so hopefully that fixes the line endings. I wonder if that could be related to the build failure?

@NasAmin
Copy link
Owner

NasAmin commented Aug 12, 2021

No the failure seems to be due to an issue with the checks API and the permissions for the default GITHUB_TOKEN. I'll look into that later which will hopefully unblock your PR.

@flcdrg
Copy link
Contributor Author

flcdrg commented Aug 12, 2021

Oh.. that sounds similar to what I've been dealing with in regards to dependabot PRs. GitHub now treats builds from forks differently. The GITHUB_TOKEN is readonly, so my build won't have permission to update the check.

It's a bit annoying. The workaround I ended up using was creating a separate workflow_run triggered workflow that ran after the original pr workflow. That workflow then runs with full access to a token. A bit of messing around though!

@NasAmin
Copy link
Owner

NasAmin commented Aug 12, 2021

@flcdrg I have read through and it does seem like an issue on PRs from Forks. I'll fix this in due time but for now I don't see any problems with your PR. I'll approve and merge if you agree?

@flcdrg
Copy link
Contributor Author

flcdrg commented Aug 12, 2021

Sounds good to me!

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.

2 participants