Conversation
This replaces the section on the community page. The information from the community page is not only moved into the new contributor's guide, but also updated to the current realities. Plus, a lot of new information is added, like a more helpful introduction for new contributors, a coding style guide, guidelines about handling PRs, and more. I tried to write down what I percieve as the tribal knowledge. However, people might have different versions of this tribal knowledge in their heads, so please discuss during the review what we want to codify here. Signed-off-by: beorn7 <beorn@grafana.com>
Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
|
Note that parts of this assume that prometheus/proposals#65 will be accepted. If not the required changes shouldn't be to hard to make, but will require some lengthier descriptions. |
beorn7
left a comment
beorn7
left a comment
There was a problem hiding this comment.
Thank you very much for this epic work. Looks quite good already.
In the same PR, we should update https://github.com/prometheus/docs/blob/main/src/app/community/community.md to not contain its own "Contributing" section anymore but point to this guide. Also, remove all the developer channels from that doc and the section about the dev summit.
Separately from this PR, all the CONTRIBUTING.md files in all repos should be updated to point to this guide.
docs/guides/contributing.md
Outdated
| [`#prometheus-dev:matrix.org`](https://app.element.io/#/room/#prometheus-dev:matrix.org). | ||
| In principle, IRC/Matrix is the preferred chat-like communication channel | ||
| because it is much more open than Slack. However, in practice the traffic on | ||
| IRC is very low. |
There was a problem hiding this comment.
As sad as it is, but "very low" is a euphemism here. For a very long time, the only traffic on #prometheus-dev was spam.
I would either remove the IRC section entirely, or state explicitly that it is effectively unused at the moment (which avoids confusion if people stumble upon the channel).
There was a problem hiding this comment.
I removed it for now. No reason to add more information here that has limited value for the task at hand.
docs/guides/contributing.md
Outdated
| https://developercertificate.org/ for _that_ particular contribution. | ||
|
|
||
| Once you have a change you would like to propose, push it to your personal fork | ||
| of Prometheus and open a pull request against the `main` branch. |
There was a problem hiding this comment.
Maybe here is the right place to explain the subtleties (release branches, master branches).
There was a problem hiding this comment.
I added a little bit, not sure if all subtleties are in there for this context here.
| Reviewers might request changes, which requires the author to either add commits | ||
| or rewrite the existing commits. Prometheus defaults to merging PR commits (not | ||
| rebase or squash), so adding a list of `fixup` commits is not a good idea unless | ||
| they get rebased or squashed by the PR author. Rebase and squash rewrite gits | ||
| commit history and authors should be aware of the implications (for example see | ||
| https://git-scm.com/book/en/v2/Git-Branching-Rebasing). Rebasing a branch for a | ||
| pull request is generally fine. Only once others have changes based on commits | ||
| that are not part of the `main` branch yet, commit authors should refrain from | ||
| rewriting those commits. |
There was a problem hiding this comment.
While I personally agree with the workflow as described here, it is not really representing the common practice in the project.
Many PRs evolve with a ton of fixup commits, and the reviewer squashes them all into one big commit in the end (or asks the author to group the commit into something reasonable if the PR changes different things that should be in separate commits).
I think this is totally fine as a practice, as long as nobody else has based work on the commits in the review branch. And as such, we should represent here as a valid workflow. (We should, though, include here that the reviewer squashing the commits needs to make sure we get a proper commit message, not the autogenerated one you see after hitting "squash" in the GH web UI.)
| that are not part of the `main` branch yet, commit authors should refrain from | ||
| rewriting those commits. | ||
|
|
||
| ## AI generated contributions |
There was a problem hiding this comment.
If we add an AI policy, as discussed on Slack, this section should not duplicate it but simply link to it.
The alternative would be to make this section our AI policy. In that case, we should include (and emphasize) the aspect of the human author being able to understand what they are submitting (see discussion on Slack).
Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
2 participants
No description provided.