vscode-gitlens icon indicating copy to clipboard operation
vscode-gitlens copied to clipboard

Converts GitHub integration authentication to use gk.dev GLVSC-554

Open sergeibbb opened this issue 1 year ago • 2 comments

Description

Introduces a provider for GitHub integration that tries to check for existing GitHub session before accessing an integration on GKDev (GLVSC-554)

Checklist

  • [x] I have followed the guidelines in the Contributing document
  • [x] My changes follow the coding style of this project
  • [x] My changes build without any errors or warnings
  • [x] My changes have been formatted and linted
  • [x] My changes include any required corresponding changes to the documentation (including CHANGELOG.md and README.md)
  • [ ] My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • [ ] My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

sergeibbb avatar Jun 18 '24 13:06 sergeibbb

Hey @sergeibbb, while reviewing your PR, I'd suggest the following code changes:

👉 Fixes integration.connect throwing error for GitHub

Ensures that manageCloudIntegrations is always called before attempting integration.connect for GitHub

You can also review and apply these suggestions locally on your machine.

Learn more about GitKraken Code Suggest

Code Suggest liberates your code reviews from GitHub's restrictive, comment-only feedback style. As simple as suggesting changes in a Google-doc, provide real code suggestions from where you code, e.g. your IDE, and on anything in your project — not just on the lines of code changed in the PR.

Join your team on GitKraken to speed up PR review.

axosoft-ramint avatar Jun 25 '24 21:06 axosoft-ramint

@sergeibbb There are issues with integration.connect being called on a cloud integration before it is ready. manageCloudIntegrations should always be called before attempting the connection, or the user may not be logged in or may not see the settings page first.

I made some updates to address this - see my code suggestion above.

axosoft-ramint avatar Jun 25 '24 21:06 axosoft-ramint