Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

sg: when in CI we do not need to prompt for an identity#63712

Merged
burmudar merged 7 commits into
mainfrom
wb/sg/no-ident-prompt-ci
Jul 9, 2024
Merged

sg: when in CI we do not need to prompt for an identity#63712
burmudar merged 7 commits into
mainfrom
wb/sg/no-ident-prompt-ci

Conversation

@burmudar

@burmudar burmudar commented Jul 9, 2024

Copy link
Copy Markdown
Contributor

There are cases when we use SG in CI and then we do not want to prompt for identity

Test plan

CI, unit tests and tested locally

Changelog

@burmudar burmudar self-assigned this Jul 9, 2024
@cla-bot cla-bot Bot added the cla-signed label Jul 9, 2024
@burmudar burmudar requested a review from a team July 9, 2024 11:12
Comment thread dev/sg/internal/analytics/oauth.go Outdated
Comment thread dev/sg/internal/analytics/oauth.go Outdated
@burmudar burmudar marked this pull request as draft July 9, 2024 11:20
Comment thread dev/sg/internal/analytics/analytics_test.go Outdated

@jhchabran jhchabran 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.

Why wouldn't we handle that case at the toplevel? I feel it would be simpler For example, we could say that the flag that disables it is always enabled it we see CI=true ?

@burmudar

burmudar commented Jul 9, 2024

Copy link
Copy Markdown
Contributor Author

Why wouldn't we handle that case at the toplevel? I feel it would be simpler For example, we could say that the flag that disables it is always enabled it we see CI=true ?

Yeah I think is a good point tbh. Otherwise it becomes really nasty in the test to enable, disable and restore the CI env var

@burmudar burmudar force-pushed the wb/sg/no-ident-prompt-ci branch from b7f3db4 to 5254c88 Compare July 9, 2024 13:17
@burmudar burmudar marked this pull request as ready for review July 9, 2024 13:18
Comment thread dev/sg/main.go Outdated
@burmudar burmudar requested a review from jhchabran July 9, 2024 13:19
@burmudar burmudar merged commit 5fd7947 into main Jul 9, 2024
@burmudar burmudar deleted the wb/sg/no-ident-prompt-ci branch July 9, 2024 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants