You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: CONTRIBUTING.md
+16-3Lines changed: 16 additions & 3 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -659,9 +659,11 @@ This goes hand in hand with [Writing good commit messages](#writing-good-commit-
659
659
660
660
For the code quality assessment, you cannot do anything yourself as only the committer can do this and they already have your code to look at.
661
661
In order to minimise the need for back and forth though, do take a look over your code changes yourself and try to put yourself into the shoes of someone who didn't just write that code.
662
-
Would you immediately know what the code does by glancing at it?
662
+
Would you immediately know what the code does or why it is needed by glancing at it?
663
663
If not, reviewers will notice this and will ask you to clarify the code by refactoring it and/or adding a few explanations in code comments.
664
664
Doing this preemptively can save you and the committer a lot of time.
665
+
To better convey the "story" of your change, consider dividing your change into multiple atomic commits.
666
+
There is a balance to strike however: over-fragmentation causes friction.
665
667
666
668
The code artefacts are the hardest for committers to assess because PRs touch all sorts of components: applications, libraries, NixOS modules, editor plugins and many many other things.
667
669
Any individual committer can only really assess components that they themselves know how to use however and yet they must still be convinced somehow.
@@ -689,7 +691,9 @@ Ask them nicely whether they still intend to review your PR and find yourself an
689
691
690
692
### How can I get a committer to look at my PR?
691
693
692
-
- Simply wait. Reviewers frequently browse open PRs and may happen to run across yours and take a look.
694
+
- Improve skimmability: use a simple descriptive PR title (details go in commit titles) outlining _what_ is done and _why_.
695
+
- Improve discoverability: apply all relevant labels, tick all relevant PR body checkboxes.
696
+
- Wait. Reviewers frequently browse open PRs and may happen to run across yours and take a look.
693
697
- Get non-committers to review/approve. Many committers filter open PRs for low-hanging fruit that are already been reviewed.
694
698
-[@-mention](https://github.blog/news-insights/mention-somebody-they-re-notified/) someone and ask them nicely
695
699
- Post in one of the channels made for this purpose if there has been no activity for at least one week
@@ -730,6 +734,13 @@ There may be constraints you had to work with which they're not aware of or qual
730
734
731
735
There are some further pitfalls and realities which this section intends to make you aware of.
732
736
737
+
### Aim to reduce cycles
738
+
739
+
Please be prepared for it to take a while before the reviewer gets back to you after you respond.
740
+
This is simply the reality of community projects at the scale of Nixpkgs.
741
+
As such, make sure to respond to _all_ feedback, either by applying suggested changes or argue in favor of something else or no change.
742
+
It wastes everyone time waiting for a couple of days just for the reviewer to remind you to address something they asked for.
743
+
733
744
### A reviewer requested a bunch of insubstantial changes on my PR
734
745
735
746
The people involved in Nixpkgs care about code quality because, once in Nixpkgs, it needs to be maintained for many years to come.
@@ -746,7 +757,7 @@ Their intent is not to denounce your code, they simply want your code to be as g
746
757
Through their experience, they may also take notice of a seemingly insignificant issues that have caused significant burden before.
747
758
748
759
Sometimes however, they can also get a bit carried away and become too perfectionistic.
749
-
If you feel some of the requests are unreasonable or merely a matter of personal preference, try to nicely remind the reviewers that you may not intend this code to be 100% perfect or that you have different taste in some regards and press them on whether they think that these requests are *critical* to the PR's success.
760
+
If you feel some of the requests are unreasonable, out of scope, or merely a matter of personal preference, try to nicely remind the reviewers that you may not intend this code to be 100% perfect or that you have different taste in some regards and press them on whether they think that these requests are *critical* to the PR's success.
750
761
751
762
While we do have a set of [official standards for the Nix community](https://github.com/NixOS/rfcs/), we don't have standards for everything and there are often multiple valid ways to achieve the same goal.
752
763
Unless there are standards forbidding the patterns used in your code or there are serious technical, maintainability or readability issues with your code, you can insist to keep the code the way you made it and disregard the requests.
@@ -771,6 +782,8 @@ Please see it as your responsibility to actively remind reviewers of your open P
771
782
The easiest way to do so is to simply cause them a Github notification.
772
783
Github notifies people involved in the PR when you add a comment to your PR, push your PR or re-request their review.
773
784
Doing any of that will get you people's attention again.
785
+
Everyone deserves proper attention, and yes that includes you!
786
+
However please be mindful that committers can sadly not always give everyone the attention they deserve.
774
787
775
788
It may very well be the case that you have to do this every time you need the committer to follow up upon your PR.
776
789
Again, this is a community project so please be mindful of people's circumstances here; be nice when requesting reviews again.
0 commit comments