Add docs on creating sandbox repos #125

evankanderson · 2020-04-15T23:42:18Z

Fixes #152 , #76

Since I've had to do it twice, I bothered writing up some instructions.

/assign @mattmoor

mattmoor

This feels redundant with some docs in test-infra (cc @chaodaiG @chizhg for the best pointer).

mechanics/CREATING-A-SANDBOX-REPO.md

mattmoor · 2020-04-16T03:44:13Z

mechanics/CREATING-A-SANDBOX-REPO.md

+   `config/prow/config_knative.yaml` and then running
+   `./hack/generate-configs.sh`
+
+1. Set up the "Settings" on the repo as follows:


This requires repo or org admin.

mattmoor · 2020-04-16T03:44:56Z

mechanics/CREATING-A-SANDBOX-REPO.md

+     - set "@knative-admin" group as "Admin" role
+     - set "\${WG}-writers" group as "Write" role
+   - Under "Branches" add a branch protection rule for `master`:
+     - Require status checks to pass (except `...-go-coverage` checks)


only the pull- legs should be required, though technically I think only "tide" is truly required.

Technically only tide is required, and I can't remember if test-infra is trying to make that change, but it's often helpful to me as a reviewer to see a Required label next to each check that tide would require.

chaodaiG · 2020-04-16T14:18:22Z

This feels redundant with some docs in test-infra (cc @chaodaiG @chizhg for the best pointer).

This is the only section that I found redundant, but i think it has its value to be presented here:

   using the automation, which probably involves updating
   `config/prow/config_knative.yaml` and then running
   `./hack/generate-configs.sh`

…sed-due-to-coronavirus

evankanderson · 2020-04-23T20:43:35Z

Clarified the two places mentioned -- one on repo access, and one that the link actually points to the automation instructions.

lberk · 2020-04-29T17:38:36Z

mechanics/CREATING-A-SANDBOX-REPO.md

+
+The [`knative-sandbox`](https://github.com/knative-sandbox) GitHub org exists to
+hold "non-core" Knative components which are owned and sponsored by a Working
+Group but are not part of an official Knative release.


Is this strictly true? @n3wscott is looking at moving camel & couchdb sources into sandbox (due to dep issues) and both of those were part of the release assests for eventing-contrib

I think @n3wscott's plan is currently a tactical maneuver, but it looks like there is also a proposal to migrate most of these repos, so either couchdb/camel-k will move back to the knative repo, or all the other contents will be moved to -sandbox as well.

right -- and it's if the latter case occurs I'm wondering if that line is strictly true or not

lberk · 2020-04-29T17:40:05Z

mechanics/CREATING-A-SANDBOX-REPO.md

+   https://github.com/knative-sandbox using the "New" button. Set the repo to
+   public and include an "Apache License 2.0" but no `.gitignore` or `README`.
+
+1. (Requires repo write/org owner) Create an `OWNERS` file and `README.md` for


should we specify that eventing OWNERS also be added this for maintenance purposes?

This is a general guide for adding things in knative-sandbox, not eventing specific.

aha good point -- then perhaps it's worthwhile to inherit the appropriate OWNERs from whichever WG Lead (technical or execution) approved the request.

That's why I didn't specify exactly what went in the OWNERS file. This is notes for a human to follow, not an automated script. 😁

vaikas · 2020-05-07T22:18:12Z

mechanics/CREATING-A-SANDBOX-REPO.md

+     - set "\${WG}-writers" group as "Write" role
+   - Under "Branches" add a branch protection rule for `master`:
+     - Require status checks to pass (except `...-go-coverage` checks)
+     - Include administrators


Do we also want to require this?

Require pull request reviews before merging When enabled, all commits must be made to a non-protected branch and submitted via a pull request with the required number of approving reviews and no changes requested before it can be merged into a branch that matches this rule.

I think that prow / tide + status checks is probably sufficient. As precedent, this is not set on serving.

I think this is part of the GitHub review flow, and we're using /lgtm and /approve instead of "changes requested" reviews.

When this is enabled, there's a "merge is blocked" message in the merge button unless someone clicks the approve radio button in a review. But Prow robot can merge anyway since it's an admin.

vaikas · 2020-05-07T22:40:46Z

mechanics/CREATING-A-SANDBOX-REPO.md

+
+1. Set up
+   [test-infra using the automation](https://github.com/knative/test-infra/blob/master/guides/prow_setup.md#setting-up-prow-for-a-new-repo-reviewers-assignment-and-auto-merge)
+   , which probably involves updating `config/prow/config_knative.yaml` and then


this is config/prod/prow/config_knative.yaml

(note the missing /prod/ in there)

Fixed, thanks!

vaikas · 2020-05-07T23:48:02Z

mechanics/CREATING-A-SANDBOX-REPO.md

+  `knative.dev` import paths for golang.
+
+- Prow automation for tests is not necessarily required for `knative-sandbox`,
+  but the Google CLA bot and OWNERS files/tide merge should be enforced.


How do you set up the CLA bot?

So, Google owns all sandbox projects?

Can we switch to a DCO for these? Or at least allow for it

I think you'll need to take that to the TOC -- Google owns the trademark on "Knative", and the current requirements from #23 are to have the Google CLA-bot enforced.

The CLA-bot is set up on the org as a whole.

I think Ville discovered that you can't select which checks are enforced until they've been run on at least one PR, so I moved the branch-protection setup to an additional item after creating a test PR for Prow.

The details are in the CLA which is a pretty long read. The Istio team wrote this very short summary about what it means: "...is necessary because you own the copyright to your changes, even after your contribution becomes part of this project. So this agreement simply gives Google permission to use and redistribute your contributions as part of the project." https://github.com/istio/community/blob/master/CONTRIBUTING.md#contributor-license-agreements

Doug, can you add the trademark question to the overall trademark issue: #9

Can we just switch to DCO for everything?

No, Google projects require the Google CLA.

The Google CLA has language like You hereby grant to Google and to recipients of software distributed by Google.... does this imply that "Google is distributing Knative" and not the community? Does it mean Google and the community are one in the same?

duglin · 2020-05-10T00:13:59Z

mechanics/CREATING-A-SANDBOX-REPO.md

+
+1. (Requires org or repo admin) Set up the "Settings" on the repo as follows:
+
+   - Disable "wiki"


I would think this would be something they might choose to enable on a per repo basis

I think we've been discouraging wiki because the access control and review is weaker, and it's easier to end up with content that could lead a user to an insecure setup.

duglin · 2020-05-10T00:14:59Z

Should we include something about governance, even if it just points to the knative docs for that?

evankanderson · 2020-05-12T18:53:28Z

Should we include something about governance, even if it just points to the knative docs for that?

We should probably have at least one template file, maybe CODE-OF-CONDUCT.md that references the governance and COC. That might be a good option for the first PR.

(Comment fail, correcting)

evankanderson · 2020-05-14T17:32:02Z

Besides the CLA questions, any other concerns with putting these directions in the repo with the following change I made the other day:

Add a reference to the community docs via CODE-OF-CONDUCT.md
Do the C-O-C as a PR to test Prow checks and get the checks to show up in the UI
Move the check enforcement after the PR is created, since it can't be applied before then.

evankanderson · 2020-05-26T17:36:51Z

Ready for another review or /lgtm.

rhuss · 2020-06-19T06:32:55Z

mechanics/CREATING-A-SANDBOX-REPO.md

+
+## Process
+
+1. (Requires Org owner) Create the new repo in


@evankanderson what is the concrete process for the communication between the requestor and the org owner ? Should this go via slack, issue, mailing-list ? I'd suggest to use a GitHub issue for this in the community repo, so maybe one can mention this here, too ?

I think am issue template with a check list would be very helpful, too.

Good point, I'll see about adding an issue template for this which includes these steps.

…sed-due-to-coronavirus

grantr · 2020-06-26T23:22:47Z

I went through these instructions for knative/eventing#3167. Here are my friction log notes:

Took about 1.5 hours to get through all the steps. With no templates or clear examples for most of it, I had to go looking for them.

Could have saved a lot of time by copying a template repo with some standard files already in place:

LICENSE
OWNERS_ALIASES containing TOC members
OWNERS including TOC alias as approvers
README.md
CODE-OF-CONDUCT.md

And then web-editing those as needed.

Confirmed with @chaodaiG that the test-infra steps at https://github.com/knative/test-infra/blob/master/guides/prow_knative_setup.md#setting-up-prow-for-a-new-repo-reviewers-assignment-and-auto-merge are still current for creating a new repo.

It's unclear what the standard Prow job template (presubmit and continuous) is for an empty repo with no tests. I copied the jobs from other repos, but example yaml blocks would be clearer.

I added build, unit, and integration presubmit jobs since it seemed better to assume that the new repo owners would eventually add these tests. To verify these jobs, I created the first PR with an no-op test/presubmit-tests.sh (just calls true) and checked that they succeeded.

I forked the repo to create this first PR, but next time I'll just create a new branch in the same repo and use web editing. In general, using GitHub's web UI exclusively (without cloning anything locally) would have saved a bunch of time.

When I created the first PR, Prow started commenting and adding labels even though none of my supporting PRs were merged. Seems like Prow approval mechanisms work on all repos in the org once they have the Prow robot added as an admin. I still waited for the test-infra changes to go in because I wanted to see the presubmit jobs pass, but I think I could have gotten a PR merged by Tide before then.

Because I put test/presubmit-tests.sh in the first PR, it didn't seem necessary to have another PR to add CODE-OF-CONDUCT.md. This file will be the same for every repo so it should be part of the template.

In some repos, CODE-OF-CONDUCT.md contains a link to the community repo, in others, a copy. Probably best to link so we don't have multiple versions floating around.

I copied branch protection rules from the eventing repo, but a canonical example would be better. Maybe the template repo can also serve as an example of branch protection rules and collaborator access (like prow robot).

I created a bunch of PRs in various repos, who should I request review from? Probably fine to let Prow suggest someone in the future, but in this case I cc'd @evankanderson on each one so he can see if everything is done right.

mechanics/CREATING-A-SANDBOX-REPO.md

mattmoor

couple nits, and an observation, but we can iterate on the peribolos observation to avoid churning this more.

mattmoor · 2020-07-15T16:59:37Z

.github/ISSUE_TEMPLATE/question.md

+name: Question
+about: Question for TOC or Steering Committee
+title: ""
+---


We should think about labeling these in the template, so that we can query them?

That sounds like a good but orthogonal project.

It might be worth seeing if we can get @macruzbar to summarize some triage best-practices gleaned from across the WGs, so we don't need to design our own.

mattmoor · 2020-07-15T17:01:55Z

mechanics/CREATING-A-SANDBOX-REPO.md

+## Process (to be executed by TOC or Steering member)
+
+1. (Requires Org owner) Create the new repo in
+   https://github.com/knative-sandbox using the "New" button. Set the repo to


@tcnghia and I learned the hard way yesterday that peribolos can create skeleton repos for you, so unless we're starting from a template repo, that might be a useful trick to incorporate.

I think a set of template repos is probably the way to go.

base --> controller
--> source
--> ???

But I'm worried about building more than we need, so let's start with the sample-controller and sample-source, plus a barebones one, and see where we get.

grantr

Thanks @evankanderson!

/lgtm

grantr · 2020-07-15T17:01:40Z

mechanics/CREATING-A-SANDBOX-REPO.md

+     - set "@knative-admin" group as "Admin" role
+     - set "\${WG}-writers" group as "Write" role
+   - Under "Branches" add a branch protection rule for `master`:
+     - Require status checks to pass (except `...-go-coverage` checks)


Technically only tide is required, and I can't remember if test-infra is trying to make that change, but it's often helpful to me as a reviewer to see a Required label next to each check that tide would require.

grantr · 2020-07-15T17:03:23Z

mechanics/CREATING-A-SANDBOX-REPO.md

+     - set "\${WG}-writers" group as "Write" role
+   - Under "Branches" add a branch protection rule for `master`:
+     - Require status checks to pass (except `...-go-coverage` checks)
+     - Include administrators


When this is enabled, there's a "merge is blocked" message in the merge button unless someone clicks the approve radio button in a review. But Prow robot can merge anyway since it's an admin.

evankanderson · 2020-07-15T19:38:58Z

Ugh, the issue templates aren't in a directory I have OWNERS on. Looking for someone from steering to approve that part.

bsnchan · 2020-07-27T21:13:44Z

/approve

Thanks for putting this together!

knative-prow-robot · 2020-07-27T21:13:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsnchan, evankanderson, grantr

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~~ [bsnchan]

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

Add docs on creating sandbox repos

a52faad

knative-prow-robot assigned mattmoor Apr 15, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 15, 2020

knative-prow-robot requested review from mattmoor and vaikas April 15, 2020 23:42

knative-prow-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 15, 2020

mattmoor reviewed Apr 16, 2020

View reviewed changes

Evan Anderson added 3 commits April 23, 2020 13:40

Merge remote-tracking branch 'upstream/master' into sandboxes-are-clo…

24d9664

…sed-due-to-coronavirus

Address one comment from mattmoor

e3eef13

Make it clear that the link points to automation instructions

3d03d04

lberk reviewed Apr 29, 2020

View reviewed changes

vaikas reviewed May 7, 2020

View reviewed changes

vaikas mentioned this pull request May 8, 2020

Set up prow correctly knative-extensions/eventing-rabbitmq#1

Closed

duglin reviewed May 10, 2020

View reviewed changes

Address vaikas and duglin comments

1daa653

Harwayne mentioned this pull request May 28, 2020

Create knative-sandbox repo named eventing-kafka-broker #143

Closed

evankanderson mentioned this pull request Jun 18, 2020

Document process to create new repos in knative orgs #76

Closed

rhuss reviewed Jun 19, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into sandboxes-are-clo…

5f95dc5

…sed-due-to-coronavirus

knative-prow-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 22, 2020

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 22, 2020

mattmoor reviewed Jun 22, 2020

View reviewed changes

evankanderson force-pushed the sandboxes-are-closed-due-to-coronavirus branch from c2f7675 to 427687d Compare June 22, 2020 22:03

mattmoor reviewed Jun 22, 2020

View reviewed changes

Add issue templates and update repo creation directions

f8bd578

evankanderson force-pushed the sandboxes-are-closed-due-to-coronavirus branch from 427687d to f8bd578 Compare June 22, 2020 22:05

mattmoor reviewed Jun 22, 2020

View reviewed changes

mattmoor-bot

c0ffbe5

This was referenced Jun 25, 2020

Create Knative Integration Testing repo. knative/eventing#3166

Closed

Create Discovery API repo knative/eventing#3167

Closed

rhuss mentioned this pull request Jul 2, 2020

Where to add Knative client plugins ? #174

Closed

grantr reviewed Jul 9, 2020

View reviewed changes

mechanics/CREATING-A-SANDBOX-REPO.md Outdated Show resolved Hide resolved

Add additional examples and notes for repo setup.

f8859e8

mattmoor reviewed Jul 15, 2020

View reviewed changes

grantr approved these changes Jul 15, 2020

View reviewed changes

knative-prow-robot assigned grantr Jul 15, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2020

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 27, 2020

knative-prow-robot merged commit 8a248c4 into knative:master Jul 27, 2020


		1. (Requires org or repo admin) Set up the "Settings" on the repo as follows:

		- Disable "wiki"

Add docs on creating sandbox repos #125

Add docs on creating sandbox repos #125

Conversation

evankanderson commented Apr 15, 2020 • edited Loading

mattmoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chaodaiG commented Apr 16, 2020

evankanderson commented Apr 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duglin May 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evankanderson May 12, 2020 • edited Loading

Choose a reason for hiding this comment

duglin commented May 10, 2020

evankanderson commented May 12, 2020

evankanderson commented May 14, 2020

evankanderson commented May 26, 2020

Choose a reason for hiding this comment

rhuss Jun 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantr commented Jun 26, 2020

mattmoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evankanderson commented Jul 15, 2020

bsnchan commented Jul 27, 2020

knative-prow-robot commented Jul 27, 2020

evankanderson commented Apr 15, 2020 •

edited

Loading

duglin May 10, 2020 •

edited

Loading

evankanderson May 12, 2020 •

edited

Loading

rhuss Jun 19, 2020 •

edited

Loading