FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Set externalRepoAuth by robertbrignull · Pull Request #397 · github/codeql-action · GitHub

Set externalRepoAuth - #397

Merged
robertbrignull merged 1 commit into
mainfrom
robertbrignull/runner_auth
Feb 17, 2021
Merged

robertbrignull merged 1 commit into
mainfrom
robertbrignull/runner_auth

Conversation

Copy link
Copy Markdown
Contributor

@aeisenberg, I noticed in #395 I unfortunately don't think it'll have the effect we wanted. I believe we wanted the standard token input to be used for normal auth and the external repository token. Currently that means you need to set it for both fields. I realise this is a bit error prone. If you have ideas for how to improve this situation do please go ahead.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Copy Markdown
Contributor

I missed that. Thanks for catching. I'll take this over.

aeisenberg force-pushed the robertbrignull/runner_auth branch from f04b21a to a265353 Compare February 17, 2021 16:31

Copy link
Copy Markdown
Contributor

I think this solution is fine. Another option would be to fall back on the default API key here if externalRepoAuth, but I don't think that's any better than what you have.

Copy link
Copy Markdown
Contributor Author

Thanks for pushing the updated JS. I could have swore I ran the build but apparently not.

Another option would be to fall back on the default API key here if externalRepoAuth

Yeah that's another option. The only downside of that is we then have to implement the fallback in each place that uses it.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL