KEMBAR78
Nudge users towards sign-in by alexr00 · Pull Request #4833 · microsoft/vscode-pull-request-github · GitHub
Skip to content

Nudge users towards sign-in #4833

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 22, 2023
Merged

Nudge users towards sign-in #4833

merged 1 commit into from
May 22, 2023

Conversation

@alexr00
Copy link
Member

@alexr00 alexr00 commented May 22, 2023

Fixes #4756

@alexr00 alexr00 enabled auto-merge (squash) May 22, 2023 14:48
@alexr00 alexr00 self-assigned this May 22, 2023
rzhao271
rzhao271 approved these changes May 22, 2023
View reviewed changes
Copy link
Contributor

@rzhao271 rzhao271 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

What happens when undefined is passed to credentialStore? Does the user receive a notification or sign-in dialog?

@alexr00 alexr00 merged commit 513a2f8 into main May 22, 2023
@alexr00 alexr00 deleted the alexr00/issue4756 branch May 22, 2023 14:59
@alexr00
Copy link
Member Author

alexr00 commented May 23, 2023

@rzhao271 you mean here https://github.com/microsoft/vscode-pull-request-github/pull/4833/files#diff-04bba6a35cad1c794cbbe677678a51de13441b7a6ee8592b7b50be1f05c6f626R338 ?

The value of telemetry should never be undefined at that point.

@rzhao271
Copy link
Contributor

Woops, I forgot to tag the line. I meant https://github.com/microsoft/vscode-pull-request-github/pull/4833/files#diff-04bba6a35cad1c794cbbe677678a51de13441b7a6ee8592b7b50be1f05c6f626R344

@alexr00
Copy link
Member Author

alexr00 commented May 24, 2023

The argument that credentialStore takes is just the vscode.AuthenticationGetSessionOptions, which is just passed on to the VS Code getSession API. The behavior will be as outlined in https://github.com/microsoft/vscode/blob/38f024adf8e43c53a56c42cdb0d5bcd4697b376e/src/vscode-dts/vscode.d.ts#L15662-L15665. So, undefined means that a badge will show on the Accounts icon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rzhao271 rzhao271 rzhao271 approved these changes

Assignees

@alexr00 alexr00

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nudge users towards sign-in

3 participants

@alexr00 @rzhao271