-
-
Notifications
You must be signed in to change notification settings - Fork 139
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
Set ${{ github.token }} as a default value for githubToken #210
base: main
Are you sure you want to change the base?
Conversation
The lint error is caused by #206 The other errors I'd expect to pass, but I wont have time to look at it until later. |
If this default is changing, the docs should also be updated to show how to disable it (i.e. so we can use a different test reporter #142). |
The default isn't changing. This is just a documentation file. I don't think there's a real relation with that issue tbh. |
Huh? It looks like the point of this PR is to change the default. I don't get it.
True, but I and others already have other test reporters set up (due to that issue), and changing this default will make our actions start double reporting test results if we don't manually remove it. |
|
I'm actually changing the default value so I may need to change the documentation. Some part of action.yml including default is not just a documentation. It changes behavior. |
Ah I didn't realise that this actually works now. The docs indeed say that it should work. When GH Actions just started out (when we created this action) we actually held all the defaults in our code (or I've been ignorant of it all this time). My bad. |
Looks like this is a breaking change for public repositories (like this very repo), because the default permissions from public PRs don't include writing the checks results. It's causing the workflows to fail with the following error:
Any ideas on how you'd solve that? |
First idea I come up with is ignoring permission error on uploading check results and make warning instead. This doesn't makes failure in most CI but this may be confusing. Another idea is postpond this change to next major release. this is safer but I don't like this. |
Changes
Closes #209
Checklist