Skip to content

Set ${{ github.token }} as a default value for githubToken#210

Open
anatawa12 wants to merge 1 commit into
game-ci:mainfrom
anatawa12:main
Open

Set ${{ github.token }} as a default value for githubToken#210
anatawa12 wants to merge 1 commit into
game-ci:mainfrom
anatawa12:main

Conversation

@anatawa12

@anatawa12 anatawa12 commented Dec 24, 2022

Copy link
Copy Markdown

Changes

  • Set ${{ github.token }} as a default value for githubToken

Closes #209

Checklist

  • Read the contribution guide and accept the code of conduct
  • Readme (updated or not needed)
  • Tests (added, updated or not needed)

@github-actions

Copy link
Copy Markdown

Cat Gif

@webbertakken

Copy link
Copy Markdown
Member

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.

@timcassell

Copy link
Copy Markdown
Contributor

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).

@webbertakken

Copy link
Copy Markdown
Member

The default isn't changing. This is just a documentation file. I don't think there's a real relation with that issue tbh.

@timcassell

Copy link
Copy Markdown
Contributor

The default isn't changing. This is just a documentation file.

Huh? It looks like the point of this PR is to change the default. I don't get it.

I don't think there's a real relation with that issue tbh.

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.

@webbertakken

Copy link
Copy Markdown
Member

Huh? It looks like the point of this PR is to change the default. I don't get it.

actions.yml is just a documentation file. This PR is only correcting that documentation to what is already happening. If I remember correctly the token is actually passed by default by GitHub Actions.

@anatawa12

Copy link
Copy Markdown
Author

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.

@webbertakken

Copy link
Copy Markdown
Member

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.

@webbertakken

webbertakken commented Dec 27, 2022

Copy link
Copy Markdown
Member

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:

Error: Resource not accessible by integration

Any ideas on how you'd solve that?

@anatawa12

Copy link
Copy Markdown
Author

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.

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.

Set default value for githubToken?

3 participants