Skip to content
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

TEP-0079: Tekton Catalog Support Tiers [Catalog Annotations] #613

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

jerop
Copy link
Member

@jerop jerop commented Jan 26, 2022

In this change, we update the tekton.dev/catalog annotation
to use a three part domain of the Catalog instead of its name
and we add a Catalog url annotation as well.

This makes it easier for users to know which Catalog the resource
came from, especially when they have other Catalogs beyond those
provided by Tekton.

Thank you @afrittoli for the idea in #599 (comment) 🙏🏾

/kind tep

@tekton-robot tekton-robot added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Jan 26, 2022
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 26, 2022
@vinamra28
Copy link
Member

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 27, 2022
@chmouel
Copy link
Member

chmouel commented Jan 27, 2022

It seems that we make the URL as the catalog UUID here while URL may change in the future or overwritten by users that may wants to freeze the repo at revision (for security/offline/internal approval or other use cases)

what about having the URL in tekton.dev/catalog-url and tekton.dev/catalog as a three part domain identifier, eg: :

  • tekton.dev/catalog: dev.tekton.catalog
  • tekton.dev/catalog-uri: "https://url/tektoncd/catalog" # github shortcut this, but i really mean the fully qualified url

@vdemeester
Copy link
Member

/hold
/assign

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2022
@tekton-robot tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 31, 2022
@jerop
Copy link
Member Author

jerop commented Jan 31, 2022

@chmouel thanks for the review and discussing further offline, updated the TEP - please take another look :)
(cc @vdemeester)

@chmouel
Copy link
Member

chmouel commented Jan 31, 2022

/lgtm

🥳

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2022
@afrittoli
Copy link
Member

/assign @bobcatfish

@tekton-robot tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 4, 2022
@jerop
Copy link
Member Author

jerop commented Feb 7, 2022

@vdemeester @vinamra28 @bobcatfish - updated the proposal, please take a look :)

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2022
@bobcatfish
Copy link
Contributor

/approve

In this change, we update the `tekton.dev/catalog` annotation
to use a three part domain of the Catalog instead of its name
and we add a Catalog url annotation as well.

This makes it easier for users to know which Catalog the resource
 came from, especially when they have other Catalogs beyond those
provided by Tekton.
@@ -8,6 +8,7 @@ authors:
- '@jerop'
- '@vdemeester'
- '@vinamra28'
- '@chmouel'
Copy link
Member Author

Choose a reason for hiding this comment

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

@chmouel added you as a co-author in this TEP - please let me know if you'd prefer not to be added 😀

Copy link
Member

Choose a reason for hiding this comment

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

SGTM :)

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2022
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Sounds good to me!
/approve

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, bobcatfish, vdemeester, vinamra28

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vdemeester
Copy link
Member

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 8, 2022
@tekton-robot tekton-robot merged commit 7e89ba7 into tektoncd:main Feb 8, 2022
@jerop jerop deleted the tep-0079-url branch June 11, 2022 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Status: UnAssigned
Development

Successfully merging this pull request may close these issues.

7 participants