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

Add capability to specify auth from env var or stdin by aeisenberg · Pull Request #391 · github/codeql-action · GitHub

/ codeql-action Public

Add capability to specify auth from env var or stdin - #391

Merged
aeisenberg merged 1 commit into
mainfrom
aeisenberg/auth-from-stdin
Feb 16, 2021
ConversationCommits (1)ChecksFiles changed
Merged

Add capability to specify auth from env var or stdin#391
aeisenberg merged 1 commit into
mainfrom
aeisenberg/auth-from-stdin

Conversation

aeisenberg commented Feb 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This commit adds two new ways of specifying GitHub auth:

  1. from the GITHUB_TOKEN environment variable
  2. from standard input

This commit does not include any documentation changes and the
descriptions of new command line options will need to be tweaked.
I will work on that later.

Merge / deployment checklist

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

adityasharad reviewed Feb 12, 2021
View reviewed changes

adityasharad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks! Generally looks sensible, suggestions are mainly about user-facing wording.

Comment thread src/util.ts
Comment thread src/util.ts Outdated
Comment thread src/util.ts
Comment thread src/util.ts Outdated
Comment thread src/runner.ts Outdated

aeisenberg commented Feb 12, 2021

Copy link
Copy Markdown
Contributor Author

All your suggestions make sense. Let me work on that.

aeisenberg force-pushed the aeisenberg/auth-from-stdin branch from c6d9112 to 2687736 Compare February 12, 2021 23:23

aeisenberg commented Feb 12, 2021

Copy link
Copy Markdown
Contributor Author

I missed an instance of --github-auth. It's added now.

robertbrignull reviewed Feb 15, 2021
View reviewed changes

robertbrignull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Haven't formally approved yet. I'll let @adityasharad do that once the requirements are nailed down. But just saying that the code here LGTM from a low-level code point of view.

aeisenberg commented Feb 15, 2021

Copy link
Copy Markdown
Contributor Author

Is there any documentation change that needs to occur? Or are the new cli options sufficient?

aeisenberg force-pushed the aeisenberg/auth-from-stdin branch from 2687736 to eb8d32d Compare February 16, 2021 16:53
Add capability to specify auth from env var or stdin
d42b8ee 
This commit adds two new ways of specifying GitHub auth:

1. from the GITHUB_TOKEN environment variable
2. from standard input

This commit does not include any documentation changes and the
descriptions of new command line options will need to be tweaked.
aeisenberg force-pushed the aeisenberg/auth-from-stdin branch from eb8d32d to d42b8ee Compare February 16, 2021 18:53
adityasharad approved these changes Feb 16, 2021
View reviewed changes

adityasharad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good. Docs team suggested we file an issue internally to update the Code Scanning help.

aeisenberg merged commit 88714e3 into main Feb 16, 2021
aeisenberg deleted the aeisenberg/auth-from-stdin branch February 16, 2021 19:26
This was referenced Feb 22, 2021
Merged
Open

JLLeitschuh commented Mar 22, 2021

Copy link
Copy Markdown

Is there a plan to issue a CVE number for this?

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

robertbrignull robertbrignull left review comments
adityasharad adityasharad approved these changes
cbraynor Awaiting requested review from cbraynor

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL