-
Notifications
You must be signed in to change notification settings - Fork 60
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
*: Pull opencontainers/project-template #20
Conversation
Signed-off-by: Chris Aniszczyk <[email protected]>
…ing-guidelines Add contributing and maintainer guidelines.
And excessive trailing newlines. These came in with fcc7f42 (Add contributing and maintainer guidelines, 2016-05-03, opencontainers#1), because we don't have Git validation yet [1]. [1]: opencontainers/project-template#2 Signed-off-by: W. Trevor King <[email protected]>
For small projects like ocitools a strict requirement would just be busywork. And the risk here (a submitted PR that would have been rejected or redirected in a leader issue) is a risk assumed by the submitter (who sunk time in the wrong direction) and not much cost to the maintainers. Signed-off-by: W. Trevor King <[email protected]>
Folks have been dragging this around from the soft limit in git-commit(1) and git.git's Documentation/SubmittingPatches without believing it. Looking at the git.git history through v2.3.4 (git log --no-merges --format=%s v2.3.4), we have 29853 commits, with 56% ≤ 50 chars and 94% ≤ 70 chars. Projects that want limits should enforce them with CI tests (e.g. runtime-spec uses git-validation, which has a soft limit at 72 and a hard limit at 90 [1]). [1]: https://github.com/vbatts/git-validation/blob/be3aee994370184fd98e455abfe0948d6f45f793/rules/shortsubject/shortsubject.go#L24-L35 Signed-off-by: W. Trevor King <[email protected]>
CONTRIBUTING: Make leader-issues optional
Small, young projects like ocitools may not have grown a test suite yet. Don't make writing a Go test suite a requirement for submitting a PR. Also allow for other test frameworks, since Go's framework may not be the best fit for all projects, which may not even include Go code. Signed-off-by: W. Trevor King <[email protected]>
…mmary-limit CONTRIBUTING: Don't specify a 50-char limit
MAINTAINERS_GUIDE: Remove trailing whitespace
CONTRIBUTING: Make the test requirements contingent on an existing suite
For runtime-spec, there are often PRs that pickup and reroll another user's commits (e.g. opencontainers/runtime-spec#337). As long as the Signed-off-by entries are there (for the DCO) and the new PR references the earlier work (to avoid maintainer confusion), I see no problem with this sort of collaboration. I thought about replacing the old wording with words like the above paragraph, but it seemed overly prescriptive. Signed-off-by: W. Trevor King <[email protected]>
CONTRIBUTING: Allow collaborative pull requests
Apart from being a sign of respect to other maintainers, it also ensures that *all* pull requests get equal amounts of review -- no matter who wrote them. Maintainers should know better than to make broken patchsets, but it's also a sign of respect to the community that all pull requests have equal treatment. Signed-off-by: Aleksa Sarai <[email protected]>
MAINTAINERS: disallow self-LGTMs
This is a proposed process for approval of new releases of specifications and projects from the OCI. The creation of this process is designed to clarify how a release gets created and who needs to sign off.
I got some feedback from folks that some motivation early in the document might be helpful for why the process encourages regular communication.
Requiring applications wait 1 week to get feedback before making a release, removing "business day" wording @cyphar, @stevvooe, and @wking were in the discussion.[1] [1] opencontainers/tob#15 (comment)
Requiring the _minimum_ process for a major release to be 3 rcs and a final release. This will establish a _minimum_ timeline of 1 month to get to a release assuming zero required changes. @stevvooe, @wking were in this discussion. opencontainers/tob#15 (comment)
Changing the release goal for projects to a "SHOULD monthly release" from the original bi-weekly. @diogomonica, @stevvooe, @mrunalp, @RobDolinMS were in that discussion opencontainers/tob#15 (comment)
Fix up the language around REJECTs so it is easier to understand. The basic premise is that a release may continue with REJECTs if 2/3 of the maintainers vote to make the release. But, the maintainers SHOULD discuss and allow time for any REJECTs to become LGTMs. Spread over two discussions: [1](https://github.com/opencontainers/tob/pull/15/files/bdfa70d70f093146bc730be2576586ec8ed57cca#r66519789) and [2](https://github.com/opencontainers/tob/pull/15/files/bdfa70d70f093146bc730be2576586ec8ed57cca#r66668148)
The intention of the voting members language is to ensure that releases can proceed even if people are unresponsive, on vacation, etc without ambiguity. This is similar to how the TOB operates. Identified by @wking here: opencontainers/tob#15 (comment)
Based on discussion with wking and mrunalp participating and Stephen Day acking in IRC: opencontainers/tob#15 (comment)
This addresses @stevvooe's concern about GitHub issues being the only medium for discussion of a reject. @wking and @philips were involved in this discussion: opencontainers/tob#15 (comment)
Projects have a happy path and a slow path. The happy path is a release with maintainers agreeing and a timeout. The slow path has rejects and quorums. Based on discussion with @wking opencontainers/tob#15 (comment)
Instead of being prescriptive provide suggestions instead for how to provide release REJECTS feedback. Based on feedback from Stephen Day and @wking.
|
||
Fork the repo and make changes on your fork in a feature branch: | ||
|
||
- If it's a bugfix branch, name it XXX-something where XXX is the number of the |
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.
Since when did we add branch naming conventions? This is silly and unnecessary.
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's a fair amount of cruft in the project template (see opencontainers/project-template#20 for some of my generalization suggestions). But I think consistency with project-template and a united effort to keep project-template sane are better than each OCI project rolling their own everything. See also @crosbymichael and @vbatts on this here and here.
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.
@wking Ok, but half of this stuff doesn't make sense.
Let's stop holding up this PR on sillyness.
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.
Let's stop holding up this PR on sillyness
I'm trying to hold it up on pan-OCI consistency. But yeah, project-template moves… slowly :p. I've filed opencontainers/project-template#27 to adjust the template on this point, and am happy to merge that in-flight PR into this one if you like the change.
@wking That is not quite how that works. The contributions to the Docker project repositories were all assigned to Docker, Inc. I guess we retain this for now but we should get this worked out. This is just a Go package. Having a secondary documentation license for the README just hinders usage with extra legal baggage. |
On Fri, Jan 06, 2017 at 02:10:03PM -0800, Stephen Day wrote:
The contributions to the Docker project repositories were all
assigned to Docker, Inc.
Is that just for employees? I don't remember transfering copyright
for my contributions. If your README contributions are copyright
Docker, Inc., we should ask them to relicense those README
contributions under Apache 2.0.
|
@wking I was under the impression that DCO implied copyright assignment, but it looks like it doesn't. Either way, I wrote this working for Docker and the README is fine under Apache. |
And remove LICENSE.docs now that nothing is under the CC license anymore. The only README contributors are Stephen (for Docker) and me (for myself). Both parties are relicensing their contributions under the Apache 2.0; me via this commit message and Stephen/Docker via: On Fri, Jan 06, 2017 at 02:36:26PM -0800, Stephen Day wrote [1]: > Either way, I wrote this working for Docker and the README is fine > under Apache. The new README copyright/license text is a cross between the old README text and Apache's recommended blurb (towards the bottom of LICENSE) with the "this file" bits and such removed. The change in listed copyright holder is intended to avoid future confusion about copyright assignment: On Fri, Jan 06, 2017 at 02:36:26PM -0800, Stephen Day wrote [1]: > @wking I was under the impression that DCO implied copyright > assignment, but it looks like it doesn't. [1]: opencontainers#20 (comment) Signed-off-by: W. Trevor King <[email protected]>
On Fri, Jan 06, 2017 at 02:36:26PM -0800, Stephen Day wrote:
Either way, I wrote this working for Docker and the README is fine
under Apache.
That's close enough to “The copyright holder for my contributions is
relicencing all of my previous README contributions under Apache 2.0”
for me ;). I've pushed beb3ba7 updating the copyright/licensing
section and removing LICENSE.docs.
|
LGTM @wking Thanks for getting this worked out! |
Anything I can do to help this along? |
@dmcgowan @aaronlehmann PTAL |
@caniszczyk Do we preserve the commit history for other projects, as well? This seems like it will greatly skew contribution numbers. |
same "printed page" as the copyright notice for easier | ||
identification within third-party archives. | ||
|
||
Copyright {yyyy} {name of copyright owner} |
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.
ummm
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 is pulled in from project-template (where it mostly matches the Apache source), and you're not supposed to update the instructions here anyway.
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.
From an official Apache project:
https://github.com/apache/mesos/blob/master/LICENSE#L190
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.
And from another Apache project:
https://github.com/apache/httpd/blob/2.4.25/LICENSE#L189
I think “just copy project-template” makes the most sense, but if you feel like you're supposed to edit the template instructions (agreeing with Mesos and disagreeing with project-template, Apache's httpd, and @thaJeztah here), I can adjust the merge commit to keep the old line instead of the project-template line.
|
||
# Copyright and license | ||
|
||
Copyright © 2016 Docker, Inc. All rights reserved, except as follows. Code is released under the [Apache 2.0 license](LICENSE.code). This `README.md` file and the [`CONTRIBUTING.md`](CONTRIBUTING.md) file are licensed under the Creative Commons Attribution 4.0 International License under the terms and conditions set forth in the file [`LICENSE.docs`](LICENSE.docs). You may obtain a duplicate copy of the same license, titled CC BY-SA 4.0, at http://creativecommons.org/licenses/by-sa/4.0/. | ||
Copyright © 2016 go-digest contributors. |
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.
copyright stays the same, you cannot just change it.
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 project doesn't have copyright assignment to Docker, and there has been confusion about that before. For example, I still hold copyright on any of my contributions (at least, those contributions which are significant enough to deserve a copyright, which may be none). Docker still holds the copyright for any of it's previous contributions, but it is just one of the go-digest contributors.
of the maintainers work is what distinguishes the good projects from the | ||
great. So please be proud of your work, even the unglamourous parts, | ||
and encourage a culture of appreciation and respect for *every* aspect | ||
of improving the project - not just the hot new features. |
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.
Interesting mix of single and double spaces after periods in this paragraph. I wouldn't hold up the PR on this, but it might be something to look at fixing in the temoplate.
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.
Yeah, I'm gradually working on fixing the template. But until those land I'd rather land these policy docs without local changes.
a vote by 66% of the current maintainers with the chief maintainer having veto power. | ||
The voting period is ten business days. Issues related to a maintainer's performance should | ||
be discussed with them among the other maintainers so that they are not surprised by | ||
a pull request removing them. |
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.
Weird line wrapping here.
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 is verbatim from project-template. I'd be in favor of PRs there to get consistent line wrapping, but would rather avoid local edits to content templated upstream.
for it. We also like to send gifts—if you're into Docker schwag, make | ||
sure to let us know. We currently do not offer a paid security bounty | ||
program, but are not ruling it out in the future. | ||
Guidelines for reporting security issues are [here][security]. |
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.
Does this link work?
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.
Yup, see it in action here.
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.
You need to fill in the copyright correctly in the LICENSE and retain the same copyright in the readme.
New projects created inside the OCI can have a copyright to the foundation but donated code/projects retain the same copyright and license.
On Tue, Jan 17, 2017 at 01:46:16PM -0800, Stephen Day wrote:
@caniszczyk Do we preserve the commit history for other projects, as
well? This seems like it will greatly skew contribution numbers.
Is there any significance to contribution numbers? Preserving the Git
history means we can pull in future project-template updates with
future calls like:
$ git pull --log git://github.com/opencontainers/project-template.git
and Git can sort out conflicts with any local changes. It also makes
copyright attribution and licensing easier to track down (because I
certainly didn't write most of the content I'm pulling in here).
I've suggested a similar approach in opencontainers/runtime-tools#274,
which is still in flight as well. I think preserving the history is
useful and see no significant downside, but *shrug*, it's your
project. If you want blame to be less useful, and future pulls from
project-template to be more work, and this whole hunk under my
Signed-off-by, it's you're project ;).
|
Closing in favor of a simpler PR. |
Seed this new project with the template, as specified in the TOB proposal [1]. My personal preference is to merge to preserve the history and make future updates easier. But I've had trouble with that in the past [2], so this commit drops the template history. Generated with: $ git remote add project-template git://github.com/opencontainers/project-template.git $ git fetch project-template $ git show --oneline project-template/master 61d73a3 (project-template/master) Merge pull request opencontainers#40 from wking/minor-patch-bullet $ git read-tree project-template/master $ git add -A . $ git checkout HEAD -- README.md $ git commit -sv [1]: https://github.com/opencontainers/tob/blob/3619df26faffa123e32a819ae32647a57fce4de2/proposals/distribution.md#governance-and-releases [2]: opencontainers/go-digest#20 (comment) Signed-off-by: W. Trevor King <[email protected]>
Generated with: $ git remote add project-template git://github.com/opencontainers/project-template.git $ git fetch project-template $ git show --oneline project-template/master 61d73a3 (project-template/master) Merge pull request opencontainers#40 from wking/minor-patch-bullet $ git merge --squash --allow-unrelated-histories project-template/master $ git checkout HEAD -- .pullapprove.yml MAINTAINERS README.md RELEASES.md $ git checkout project-template/master -- GOVERNANCE.md LICENSE $ emacs README.md CONTRIBUTING.md # unify around project-template's CONTRIBUTING.md approach $ emacs meeting.ics # update link to point at CONTRIBUTING.md#meetings $ git commit -sv I personally prefer non-squash merges to preserve history and ease future updates, but that approach has not been popular within the OCI [1,2], so I'm going with a squash-merge here. I'm sticking with the local RELEASES.md, because it uses four-space indents. I've filed [3] to upstream that change. I've also filed [4] upstreaming our local wording change from 70ba4e6 (meeting: Bump January meeting from the 3rd to the 10th, 2017-12-07, opencontainers#943). I've also fixed the GOVERNANCE.md security link in flight with [5]. I've left the other in-flight project-template changes alone [6]. I've wrapped the URL in meetings.ics to avoid [7]: Line length should not be longer than 75 characters near line opencontainers#33 Reference: RFC 5545 3.1. Content Lines [1]: opencontainers/go-digest#20 (comment) [2]: opencontainers/runtime-tools#274 (comment) [3]: opencontainers/project-template#54 [4]: opencontainers/project-template#55 [5]: opencontainers/project-template#34 [6]: https://github.com/opencontainers/project-template/pulls [7]: https://icalendar.org/validator.html Signed-off-by: W. Trevor King <[email protected]>
Settle into the OCI boilerplate with: