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

Support custom GitHub token environment variable name #112

Conversation

KeisukeYamashita
Copy link
Contributor

WHAT

If the token with $ are configured, lookup the environment variable.

WHY

For #111.
In current implementation, the tfnotify only supports GITHUB_TOKEN and users can't specify a environment variable of their own.

@KeisukeYamashita KeisukeYamashita self-assigned this Feb 18, 2022
@KeisukeYamashita KeisukeYamashita force-pushed the support-custom-github-token-env-var-name branch from ec68a58 to 13d4a8b Compare February 18, 2022 12:58
@KeisukeYamashita KeisukeYamashita linked an issue Feb 18, 2022 that may be closed by this pull request
@KeisukeYamashita KeisukeYamashita marked this pull request as ready for review February 18, 2022 13:04
@KeisukeYamashita KeisukeYamashita changed the title Support custom GItHub token environment variable name Support custom GitHub token environment variable name Feb 18, 2022
@@ -69,10 +69,11 @@ type service struct {
// NewClient returns Client initialized with Config
func NewClient(cfg Config) (*Client, error) {
token := cfg.Token
token = strings.TrimPrefix(token, "$")
if token == EnvToken {
token = os.Getenv(EnvToken)

Choose a reason for hiding this comment

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

Since now the EnvToken is used only in testing, we can remove the variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, true. Let me remove it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review, removed → e5e8b67

Signed-off-by: KeisukeYamashita <[email protected]>
Copy link

@micnncim micnncim left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@KeisukeYamashita
Copy link
Contributor Author

Thank you 💪

@KeisukeYamashita KeisukeYamashita merged commit a2897c6 into mercari:master Feb 20, 2022
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.

tfnotify support custom environment variable
2 participants