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

Add CONTRIBUTING.md #6718

Merged
merged 1 commit into from
Sep 26, 2015
Merged

Add CONTRIBUTING.md #6718

merged 1 commit into from
Sep 26, 2015

Conversation

btcdrak
Copy link
Contributor

@btcdrak btcdrak commented Sep 24, 2015

Add a CONTRIBUTING.md as per https://github.com/blog/1184-contributing-guidelines

This document was written in collaboration with @laanwj.

@paveljanik
Copy link
Contributor

Please add a line or two about Trivial tree?


The body of the pull request should contain enough description about what the patch does together with any justification/reasoning. You should include references to any discussions (for example other tickets or mailing list discussions).

- At this stage one should expect comments and review from other contributors. You can add more commits to your pull request by committing them locally and pushing to your fork until you have satisfied all feedback. If your pull request is accepted for merging, you may be asked by a maintainer to squash and or rebase your commits before it will be merged. The length of time for required for peer review is unpredictable and will vary from patch to patch.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Remove -.

@laanwj
Copy link
Member

laanwj commented Sep 24, 2015

ACK, thanks for writing this up

@paveljanik
Copy link
Contributor

ACK, very well written!

@gavinandresen
Copy link
Contributor

ACK, very nice!

@laanwj
Copy link
Member

laanwj commented Sep 24, 2015

@paveljanik Re: the trivial tree, maybe @theuni could comment on that. I think it was a great idea, but AFAIK it has not been kept up to date regularly lately, so I 'm not sure what his stance on it is now.

@paveljanik
Copy link
Contributor

If it is so, I think that adding [Trivial] to pull request titles is enough to solve this.


Commit messages should be verbose by default consisting of a short subject line (50 chars max), a blank line and detailed explanatory text as separate paragraph(s); unless the title alone is self-explanatory (like "fixed spelling") then a single title line is sufficient. Commit messages should be helpful to people reading your code in the future, so explain the reasoning for your decisions. Further explanation [here](http://chris.beams.io/posts/git-commit/).

If a particular commit references another issue, please add the reference, for example "refs #1234", or "fixes #4321". Using "fixes or closes" keywords will cause the corresponding issue to be closed when the pull request is merged.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is a good idea. Git and Github are two separate things. I agree with referencing a fix by fixes #4321 placed in a Github PR description or a Github comment. Placing it in a git commit is not required and not best practice IMO.

Copy link
Member

Choose a reason for hiding this comment

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

+1. I prefer to place it somewhere in the GitHub PR description.

Copy link
Member

@sipa sipa Sep 24, 2015 via email

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely, it must be understandable without referring to them, but please do refer to solved github issues in the commit descriptions. This makes it much easier to keep track of what was solved when when browsing history.

@jonasschnelli
Copy link
Contributor

ACK.
Nice work!

Contributor Workflow
--------------------

The codebase is maintained using the “contributor workflow” where everyone without exception contributes patch proposal using “pull requests”. This facilitates social contribution, easy testing and peer review.
Copy link
Member

Choose a reason for hiding this comment

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

s/proposal/proposals

@fanquake
Copy link
Member

Nice work. Definite improvement on the developer-notes, which were somewhat hidden away. The fact that this is shown as suggested reading to everyone opening a pull request is also good.


The Bitcoin Core project operates an open contributor model where anyone is welcome to contribute towards development in the form of peer review, testing and patches. This document explains the practical process and guidelines for contributing.

Firstly in terms of structure, there is no particular concept of “Core developers” in the sense of privileged people. Open source often naturally revolves around meritocracy where longer term contributors gain more trust from the developer community. However, some hierarchy is necessary for practical purposes. As such there are repositories “maintainers” who are responsible for merging pull requests as well as a “lead maintainer” who is responsible for overall merging, moderation and appointment of maintainers and the release cycle.
Copy link
Member

Choose a reason for hiding this comment

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

s/hierarchy/roles/

Copy link
Member

Choose a reason for hiding this comment

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

s/repositories/repository/


Project maintainers reserve the right to weigh the opinions of peer reviewers using common sense judgement and also may weight based on meritocracy: Those that have demonstrated a deeper commitment and understanding towards the project (over time) or have clear domain expertise may naturally have more weight, as one would expect in all walks of life.

Where a patch set affects consensus critical code, the bar will be set much higher in terms of discussion and peer review, remembering mistakes could be very costly to the wider community. This includes refactoring of consensus critical code.
Copy link
Member

Choose a reason for hiding this comment

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

"remembering [that] mistakes" or "keeping in mind that" perhaps?

Good work :)

@btcdrak btcdrak force-pushed the contrib branch 2 times, most recently from b024d6d to 649f544 Compare September 26, 2015 07:56
@laanwj laanwj merged commit 06d92d7 into bitcoin:master Sep 26, 2015
laanwj added a commit that referenced this pull request Sep 26, 2015
06d92d7 Add CONTRIBUTING.md (BtcDrak)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.