search

knative / build-templates

A library of build templates.
Apache License 2.0
184 stars 68 forks source link

ClusterBuildTemplate variations of each BuildTemplate file #79

Closed drnic closed 5 years ago

drnic commented 5 years ago 
$ kubectl apply -f buildpack/buildpack-cluster.yaml
clusterbuildtemplate.build.knative.dev/buildpack created
$ kubectl apply -f buildpack/buildpack.yaml
buildtemplate.build.knative.dev/buildpack created
knative-prow-robot commented 5 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: drnic To fully approve this pull request, please assign additional approvers. We suggest the following additional approver: mattmoor

If they are not already assigned, you can assign the PR to them by writing /assign @mattmoor in a comment when ready.

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: - **[OWNERS](https://github.com/knative/build-templates/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
vdemeester commented 5 years ago

/ok-to-test

drnic commented 5 years ago

Was there a reason to not merge this last year? Want me to rebase?

vdemeester commented 5 years ago

@drnic I don't think so cc @ImJasonH

imjasonh commented 5 years ago

I'm not sure how I so thoroughly missed this PR! :astonished:

I'm slightly opposed to having to maintain two copies of each build template going forward, it seems like a recipe for them getting out of sync in confusing and unobvious ways, or otherwise like a speed bump to avoid in every future PR that gets proposed.

Perhaps if there was a CI test that checked that they were identical, and a script that could be run to copy-and-update the ClusterBuildTemplate version, that would help a lot.

And in any case I think if we're going to invest in improving this catalog of build templates, we should consider making the same change instead of / in addition to the Tekton Task catalog at https://github.com/tektoncd/catalog.

Up to you if you want to move this forward, I don't feel strongly that we really need it. Maybe just a blurb somewhere that any namespaced BuildTemplate can also be installed as a ClusterBuildTemplate by changing the kind.

WDYT?

drnic commented 5 years ago

Ok, I'll close the issue for now. Someone/myself can reopen in future.