-
Notifications
You must be signed in to change notification settings - Fork 503
fix(cicd-statistics-module-github): Align authentication with other github frontends #6084
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
base: main
Are you sure you want to change the base?
fix(cicd-statistics-module-github): Align authentication with other github frontends #6084
Conversation
Changed Packages
|
|
Thanks for the contribution! |
…ithub frontends Signed-off-by: Gavin Elder <[email protected]>
acbfa60 to
6f8affc
Compare
…ithub frontends Signed-off-by: Gavin Elder <[email protected]>
awanlin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @gavinelder, just left a few minor comments 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave this one out of the changes. It's already likely out of date as we just merged a bunch of 1.45.x version bumps. It also keeps the subject of the PR focused. 👍
| @@ -0,0 +1,33 @@ | |||
| /* | |||
| * Copyright 2024 The Backstage Authors | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Copyright 2024 The Backstage Authors | |
| * Copyright 2025 The Backstage Authors |
Seeing as this is a new file
| * @packageDocumentation | ||
| */ | ||
|
|
||
| export { CicdStatisticsApiGithub, GITHUB_ACTIONS_ANNOTATION } from './api'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export { CicdStatisticsApiGithub, GITHUB_ACTIONS_ANNOTATION } from './api'; | |
| export { CicdStatisticsApiGithub } from './api'; |
Not sure why we are exporting GITHUB_ACTIONS_ANNOTATION here, seems odd to do and it's already exported by the GitHub Actions plugin.
| url: `https://${hostname}/`, | ||
| additionalScope: { | ||
| customScopes: { | ||
| github: ['repo'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm this will get us the same results as the current read_api?
| '@backstage-community/plugin-cicd-statistics-module-github': minor | ||
| --- | ||
|
|
||
| Updated Github Authentication methods to prompt for required scopes and refactored auth to align with community-plugin conventions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Updated Github Authentication methods to prompt for required scopes and refactored auth to align with community-plugin conventions | |
| Updated Github Authentication methods to prompt for required scopes and refactored auth to align with Backstage GitHub plugin conventions |
Hey, I just made a Pull Request!
The following updates the
cicd-statistics-module-githubauthentication flow to align with other backstage community plugins for example.I noticed an issue where the actions statistics was failing to load until you first navigated to another Github Plugin such as
github-actionswhich would then prompt you to authenticate with the correct scopes required for the action.When looking to update the scope checking and using the existing
github-actionscode for reference I noticed that there was a major divergence in the relevant code and decided to align it to the existing github centric modules.✔️ Checklist
Signed-off-byline in the message. (more info)