docs: Add contributing guidelines for go-tuf by asraa · Pull Request #190 · theupdateframework/go-tuf

asraa · 2021-12-09T20:37:06Z

Include PR process, review process, and other communication details.

Signed-off-by: Asra Ali asraa@google.com

Signed-off-by: Asra Ali <asraa@google.com>

coveralls · 2021-12-09T20:38:34Z

Pull Request Test Coverage Report for Build 1560595099

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 69.337%

Totals
Change from base Build 1559969196:	0.0%
Covered Lines:	2051
Relevant Lines:	2958

💛 - Coveralls

mnm678

Thanks for putting this together! I left some comments inline

mnm678 · 2021-12-10T20:21:51Z

+## Pull Request review policy
+
+* Anyone is welcome to review any PR, whether they are a maintainer or not!
+* Maintainers should aim to turn around reviews within one business day.


One day might be a bit optimistic. While this will usually be the case maintainers might take a week off, etc and I don't want to set expectations too high.

How does a 5 business days sound? Also, it's unlikely all the maintainers are off, and I think we can make exceptions for regional holidays.

mnm678 · 2021-12-10T20:22:56Z

+* See [MAINTAINERS](MAINTAINERS) for the current list of maintainers.
+* It is expected that two maintainers from differing organizations approve the PR before a merge. This may be waived for PRs which only update docs or comments, or trivial changes to tests.
+
+Maintainers should:


we can also have something here about ensuring compliance with the TUF spec and overall code quality

mnm678 · 2021-12-10T20:24:10Z

+
+To make a pull request, you will need a GitHub account. See GitHub's documentation [forking](https://help.github.com/articles/fork-a-repo) and [pull requests](https://help.github.com/articles/using-pull-requests). 
+
+Pull requests should be targeted at the `master` branch. Before creating a pull request, go through this checklist:


This section is a great addition, especially for new contributers

rdimitrov · 2021-12-13T15:26:31Z

It would be also nice if we can add a Contributing chapter to the README file too where we refer to this guideline 👍

rdimitrov · 2021-12-13T15:20:48Z

+When all of the tests are passing, maintainer(s) will be assigned to review and merge the PR. 
+
+
+## Communication


Maybe it would be nice to mention the community meeting schedule as well 👍

znewman01 · 2022-01-26T14:59:31Z

+
+## Pull Request Procedure
+
+To make a pull request, you will need a GitHub account. See GitHub's documentation [forking](https://help.github.com/articles/fork-a-repo) and [pull requests](https://help.github.com/articles/using-pull-requests). 


nit: "Github's documentation ON forking..."

znewman01 · 2022-01-26T15:01:10Z

+
+## Contributing Code
+
+Unless you are fixing a known bug, we strongly recommend discussing it with the community via a GitHub issue or Slack before getting started to ensure that your work is consistent with TUF's specification.


Maybe: ...or Slack (see [Communication](#Communication))

znewman01 · 2022-01-26T15:04:03Z

+
+When creating a PR, 
+
+1. Accept the Developer's Certificate of Origin on all commits (see above).


If we require this, probably worth adding the DCO bot

I hate that DCO bot. Can we instead use an app that checks whether a contributor has signed a ~~DCO~~ CLA agreement? Something like what the Linux Foundation uses.

🤣 🤣 I'm not attached to any particular bot, just saying that if it's "required" that should be enforced :)

Though I can't find what you're talking about -- I checked a few LF repos and the ones I found all use the same bot (ex.).

Sorry, I meant a CLA bot, which many orgs have their own apps for, here's the LF one: https://github.com/apps/lf-cla-application

TLDR: no to DCO, yes to CLA

Why the preference for CLA?

As I understand it (I am not a lawyer) both are assertions that the submitter has the right to submit a change and grants the right to the receiver to use the submitted change. A CLA is a legal contract while the DCO is a more lightweight statement of the contributor asserting ownership. As a consequence of a CLA being a legal contract, they can be a barrier to contributors (requiring, i.e., a corporate legal representative to sign-off).

Simply because DCO is extremely annoying (must remember to --sign-off, otherwise rebase and force-push hell; does not work with commits done from the website), whereas CLA is a one-time deal.

rdimitrov · 2022-03-17T11:40:32Z

In regards to #234, we should add content in the guideline for these topics too:

1. Convention for writing commit messages

Upon release, the changelog is built automatically and it will group commits based on a given regex and title.
This is especially important when we are introducing breaking changes because such will be highlighted in e separate group on top of the changelog for any new release.
The available groups will be - Breaking changes, Features, Bug fixes, and Others
To mark that a commit should be part of a given group, its commit message should have some of the following keywords - BREAKING CHANGE, feat, or fix. Everything else is added to the Others group.
The responsibility for doing that is first to the author who created these commits and second to the maintainers reviewing and thus merging them. For example, if it introduces a breaking change and the commit messages do not inform of that, the maintainer should alter the message of the merge commit upon merging it so it includes the expected keyword.

2. How to tag releases and prereleases

Use Semver-like versioning, i.e v1.2.3 and v1.2.3-rc1?

3. How to tag breaking changes

Increment the MAJOR version or something else?

@asraa - Let me know in case you are not available to spend some time on that, so we can merge this one with what we have so far and I'll address these in another PR 👍

UPDATE [03/21/22]: Updated the keyword for marking changes as breaking so it's in line with what the spec says - ref. https://www.conventionalcommits.org/en/v1.0.0/#specification

rdimitrov · 2022-04-10T16:36:06Z

Since #234 got merged, maybe we should proceed with finalising this one too, so the contributing guidelines are consistent with what the CI expects to verify 👍

rdimitrov

@znewman01 - Let's merge this as it's already a great start for having such a guideline 👍

We'll address some of the comments in follow-up PRs and I'll open a separate issue for discussing whether we want to use CLA or DCO.

znewman01 · 2022-05-16T16:00:27Z

I'd prefer to remove the contentious stuff (e.g. DCO vs. CLA) before we merge even a first-pass, so nobody confuses it for official, set-in-stone policy.

If we pull that out, I'm happy to approve it.

rdimitrov · 2022-05-17T11:26:09Z

I'd prefer to remove the contentious stuff (e.g. DCO vs. CLA) before we merge even a first-pass, so nobody confuses it for official, set-in-stone policy.

If we pull that out, I'm happy to approve it.

Got it 👍

@asraa - Would you like to address this one or if you don't have the time maybe I can fork your branch and do this on top?

@asraa

Follow-up from theupdateframework#190 (thanks @asraa!). I did not add a DCO requirement at this point, as that was controversial in theupdateframework#190. I filed theupdateframework#308 to track that. I tried to address all *other* feedback in theupdateframework#190. Fixes theupdateframework#212. Fixes theupdateframework#306.

@asraa

* add contrbuting guidelines Signed-off-by: Asra Ali <asraa@google.com> * Update CONTRIBUTING.md, add MAINTAINERS.md Follow-up from #190 (thanks @asraa!). I did not add a DCO requirement at this point, as that was controversial in #190. I filed #308 to track that. I tried to address all *other* feedback in #190. Fixes #212. Fixes #306. * Move docs into a "docs" folder. Fixes #303. * Whitespace fixes * Address PR comments - TODO for testing instructions - Remove obsolete TODO * Full URL in testing * Fix @joshuagl suggestions Co-authored-by: Asra Ali <asraa@google.com>

znewman01 · 2022-06-09T17:15:28Z

Superceded by #309

add contrbuting guidelines

b38d6bf

Signed-off-by: Asra Ali <asraa@google.com>

mnm678 reviewed Dec 10, 2021

View reviewed changes

rdimitrov reviewed Dec 13, 2021

View reviewed changes

joshuagl mentioned this pull request Jan 26, 2022

Contributing guidelines link redirects to an unrelated page #212

Closed

joshuagl linked an issue Jan 26, 2022 that may be closed by this pull request

Contributing guidelines link redirects to an unrelated page #212

Closed

znewman01 reviewed Jan 26, 2022

View reviewed changes

rdimitrov mentioned this pull request Mar 15, 2022

Add automatic releases using goreleaser #234

Merged

rdimitrov mentioned this pull request May 12, 2022

Set up versioned releases #200

Closed

rdimitrov approved these changes May 16, 2022

View reviewed changes

znewman01 mentioned this pull request Jun 8, 2022

Contributor licensing #308

Closed

znewman01 mentioned this pull request Jun 8, 2022

docs: Update CONTRIBUTING.md, add MAINTAINERS.md #309

Merged

znewman01 closed this Jun 9, 2022


		To make a pull request, you will need a GitHub account. See GitHub's documentation [forking](https://help.github.com/articles/fork-a-repo) and [pull requests](https://help.github.com/articles/using-pull-requests).

		Pull requests should be targeted at the `master` branch. Before creating a pull request, go through this checklist:

		When all of the tests are passing, maintainer(s) will be assigned to review and merge the PR.


		## Communication


		## Pull Request Procedure

		To make a pull request, you will need a GitHub account. See GitHub's documentation [forking](https://help.github.com/articles/fork-a-repo) and [pull requests](https://help.github.com/articles/using-pull-requests).


		## Contributing Code

		Unless you are fixing a known bug, we strongly recommend discussing it with the community via a GitHub issue or Slack before getting started to ensure that your work is consistent with TUF's specification.


		When creating a PR,

		1. Accept the Developer's Certificate of Origin on all commits (see above).

Conversation

asraa commented Dec 9, 2021

Uh oh!

coveralls commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 1560595099

💛 - Coveralls

Uh oh!

mnm678 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdimitrov commented Dec 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trishankatdatadog Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdimitrov commented Mar 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rdimitrov commented Apr 10, 2022

Uh oh!

rdimitrov left a comment

Choose a reason for hiding this comment

Uh oh!

znewman01 commented May 16, 2022

Uh oh!

rdimitrov commented May 17, 2022

Uh oh!

znewman01 commented Jun 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

coveralls commented Dec 9, 2021 •

edited

Loading

trishankatdatadog Jan 26, 2022 •

edited

Loading

rdimitrov commented Mar 17, 2022 •

edited

Loading