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
catalog-backend-module-github-org: fix default namespace for groups #24700
Conversation
Changed Packages
|
…Provider Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
f73e6c9
to
11c9f1f
Compare
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 a ton for this improvement @vinzscam 🚀
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
6520dc4
to
4cd526e
Compare
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.
Nice! 👍
Just a few nits
githubOrg: | ||
- id: production | ||
githubUrl: 'https://github.com', | ||
orgs: ['org-a', 'org-b'], |
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.
Ah so it wasn't possible to configure this by org after all, that explains the code. Doesn't need to be done now but prolly makes sense to add support for Array<string | { name: string, groupNamespace?: string, userNamespace?: string}>
in the future
* | ||
* If set to true, groups with the same name across different orgs will be considered the same group. | ||
*/ | ||
defaultNamespace?: boolean; |
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.
Thinking that a good name for this could be alwaysUseDefaultNamespace
?
entities: [], | ||
}) | ||
.catch(error => { | ||
console.error('Failed to clean up entities', error); |
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.
Grab the logger for this one :>
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.
🤦
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.
Docs additions look good, thanks for adding that 🚀
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
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.
Bit awkward with the option, but that'll be cleaned up eventually as we move off the old system.
Good enough I'd say, and let's 👍
Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Uffizzi Cluster |
Hey, I just made a Pull Request!
As we are suggesting adopters to use the new
catalog-backend-module-github-org
when migrating to the new backend system, this PR fixes an issue where thecatalog-backend-module-github-org
would create groups using the org as namespace instead of thedefault
namespace.Now the module uses the
default
namespace for groups, in case a single org has been configured.✔️ Checklist
Signed-off-by
line in the message. (more info)