-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Allow a domain entity to have an optional domain #17125
Allow a domain entity to have an optional domain #17125
Conversation
Changed Packages
|
Thanks for the contribution! |
Uffizzi Ephemeral Environment
|
2ebd7f8
to
5343e62
Compare
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
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.
Hi! One thing that's missing here is the corresponding update to the corresponding processor.
}, | ||
"domain": { | ||
"type": "string", | ||
"description": "An entity reference to the domain that the system belongs to.", |
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.
Need to adjust the wording here
Ping @backstage/catalog-core |
@@ -1240,6 +1240,7 @@ metadata: | |||
description: Everything about artists | |||
spec: | |||
owner: artist-relations-team | |||
domain: audio-domain |
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 are more details about the specific fields and their semantics nearby. Need to add that with the appropriate table just like other relations, explaining that the default kind is Domain and the default namespace is the same as this same domain etc.
Also might want to adjust well-known relations to mention the Domain kind around the intended relations.
Also, you call this domain
. Another candidate would have been parent
. Are you avoiding that since you intend to not use the parentOf
/childOf
relations?
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
Pinging @backstage/catalog-maintainers here again |
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
hey @freben, I think this PR is now ready for re-review! Thank you to @janogonzalez for going the last mile and finishing off my work properly 🙏 |
{ defaultKind: 'Domain', defaultNamespace: selfRef.namespace }, | ||
RELATION_PART_OF, | ||
RELATION_HAS_PART, | ||
); |
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.
this does need an additional changeset to get shipped
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.
Noted, CC @janogonzalez
… be represented. Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
Signed-off-by: Jano González <info@janogonzalez.com> Signed-off-by: Dawn James <dawnj@spotify.com>
07121b5
to
8d14475
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. Would love for @backstage/catalog-maintainers to sign off too
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
For large organisations, a hierarchy of domains might make life easier. This PR introduces an optional
domain
field into the Domain entity so that a domain can have a parent domain.Hey, I just made a Pull Request!
✔️ Checklist
Screenshots attached (for UI changes)Signed-off-by
line in the message. (more info)