submitting-changes.chapter.md: explain *why* we have three branches #225920
Conversation
ghost
requested a review
from 
There was a problem hiding this comment.
That's a very valuable addition, but I think we should restructure it a bit to make more probable that readers build the right mental model of what's going on, and we can do that by constructing a clearer narrative. Therefore I didn't go into detailed review, and while those suggestions apply in general, they may become invalid in their specific form if the text structure changes. (It would be sensible to incorporate them first and then continuing work on the contents.)
Right now the problem I see is that we're introducing at least three distinct concepts:
- mass rebuilds
- the branch structure
- merge and batch strategies
But the text begins with the branches as if it were the key concept, and otherwise lumps everything together and interleaves subjects without a clear progression. Really what the whole thing is about is an optimization of resource use, and the setup is a particular solution motivated by the phenomenon of mass rebuilds. The text should reflect that and present the information it already contains in the following order:
- what mass rebuilds are (caused by change in widely used build inputs) and why it's a problem (extreme resource use and delays in fixing broken builds, whatever, you're the expert on this)
- which solution Nixpkgs maintainers came up with (perform mass rebuilds on separate branch, batch them up on intermediate branch)
- what these branches are called and how exactly changes progress through that system
- illustration and diagram description
It probably makes sense to leave the first paragraph as the top level information one will primarily care about to look up the branch names, as a reminder, and then go into the sequence I proposed.
Also please use one line per sentence, it's much easier to make focused suggestions that way. The diffs become unwieldy quickly if lines are broken at arbitrary points.
|
This issue has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/tweag-nix-dev-update-47/27387/1 |
The manual does an okay job of explaining the rules for each of the three development branches, but really doesn't give any intuition as to why there are three (why not four? or two?) or how we got where we are today. This commit attempts to fix that, by explaining that there is one branch that allows mass-rebuild commits, and it has a fast-building branch both upstream and downstream of it (from the perspective of automated merges). I have also removed the term "stabilization" from the arc labels. This vague term is not defined anywhere, and does communicate any useful information without a longer explanation. Therefore it is not appropriate for use in a diagram. Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
|
Committed @fricklerhandwerk's suggested changes. Rebased to fix merge conflict. No other changes. |
|
Squashed. |
@fricklerhandwerk, you make several good suggestions on how to improve the prose here. However a mediocre explanation (this PR) is better than no explanation at all (which is what is currently in-tree). I've committed all of your suggested changes except the one that deleted the color legend. The remainder of your comments are suggestions on how to make the prose in this PR better, rather than indicating that the explanation is worse than no explanation at all. I'm going to merge this PR now so that I don't have to keep rebasing it. If you would like to submit a follow-up PR to improve the wording I would be happy to review it. |
ghost
mentioned this pull request
|
I panic-reverted this thinking that I had broken CI on master, but on closer inspection it seems GitHub was putting big red Xes in my notifications list for stale versions of this PR. Resubmitted as #243140 |
|
Did this intentionally revert @pennae's diff from https://github.com/NixOS/nixpkgs/pull/239636/files#diff-ba881223dddc816723e79c2ee074df7b18a08b6520273c22665143b700f3f52dL217-R221 that moved the graphviz dot file out of the markdown? The graph currently is not rendered on https://nixos.org/manual/nixpkgs/unstable/#submitting-changes-branches as a result of this change |
Why is inline graphviz not rendered anymore? |
Presumably because no more docbook, but idk exactly. If you would rather, I suppose I can make the PR that moves this graphviz to the adjacent file and restores @pennae's change |
it was the only inline graphviz figure in the entire manual, adding support for that to nixos-render-docs would've been so much more effort than it's worth. if a bunch of new users appear we may reevaluate that decision, but for now all graphviz must be rendered and referenced as svg (or any other graphviz output format, really, but preferrably svg). |
|
I'm working on moving the contributing section out of the rendered manual and into CONTRIBUTING.md (see #244056), where we can then use GitHub markdowns mermaid support. So we don't need to spend time trying to fix it in the rendered manual. |
The manual does an okay job of explaining the rules for each of the three development branches, but really doesn't give any intuition as to why there are three (why not four? or two?) or how we got where we are today. This commit tries to fix that.
I have also removed the term "stabilization" from the arc labels. This vague term is not defined anywhere, and does communicate any useful information without a longer explanation. Therefore it is not appropriate for use in a diagram.