|
| 1 | +---------------------------------------------------------------- |
| 2 | +[the structure of a log message] |
| 3 | + |
| 4 | +The usual way to compose a log message of this project is to |
| 5 | + |
| 6 | + - Give an observation on how the current system work in the present |
| 7 | + tense (so no need to say "Currently X is Y", just "X is Y"), and |
| 8 | + discuss what you perceive as a problem in it. |
| 9 | + |
| 10 | + - Propose a solution (optional---often, problem description |
| 11 | + trivially leads to an obvious solution in reader's minds). |
| 12 | + |
| 13 | + - Give commands to the codebase to "become like so". |
| 14 | + |
| 15 | +in this order. |
| 16 | + |
| 17 | +To those who have been intimately following the discussion, it often |
| 18 | +is understandable without some of the above, but we are not writing |
| 19 | +for those who review the patches. We are primarily writing for future |
| 20 | +readers of "git log" who are not aware of the review discussion we |
| 21 | +have on list, so we should give something to prepare them by setting |
| 22 | +the stage and stating the objective first, before going into how the |
| 23 | +patch solved it. |
| 24 | + |
| 25 | +------------------------------------------------------------------------ |
| 26 | +[polish your history] |
| 27 | + |
| 28 | +We frown upon a patch series that makes mistakes in an earlier step, |
| 29 | +only to fix them in a later step. The "git rebase -i" command helps |
| 30 | +us pretend to be more perfect developers than we actually are, |
| 31 | +whipping your patch series into a shape that builds one small step |
| 32 | +on top of another in a logical succession. Such a patch series is |
| 33 | +easier to understand than a history that faithfully records all the |
| 34 | +stumbles the developer made until they reached the final solution. |
| 35 | + |
| 36 | +------------------------------------------------------------------------ |
| 37 | +[not just respond, update the patches] |
| 38 | + |
| 39 | +Not limited to this one, but when a reviewer says "this is not |
| 40 | +clear", it is often not a request to only clarify something, which |
| 41 | +is clear to any intelligent user of the end product, to a clueless |
| 42 | +reviewer, whose intelligence is below the target audience, in an |
| 43 | +e-mail response. It is pointing out that the end product, either the |
| 44 | +patch text or the proposed log message, is not clear to target |
| 45 | +audience and needs update. |
| 46 | + |
| 47 | +---------------------------------------------------------------- |
| 48 | +[make us come to you, begging] |
| 49 | + |
| 50 | +I've seen from time to time people ask "I am thinking of doing this; |
| 51 | +will a patch be accepted? If so, I'll work on it." before showing |
| 52 | +any work, and my response always has been: |
| 53 | + |
| 54 | + (1) We don't know how useful and interesting your contribution would |
| 55 | + be for our audience, until we see it; and |
| 56 | + |
| 57 | + (2) If you truly believe in your work (find it useful, find writing |
| 58 | + it fun, etc.), that would be incentive enough for you to work |
| 59 | + on it, whether or not the result will land in my tree. You |
| 60 | + should instead aim for something so brilliant that we would |
| 61 | + come to you begging for your permission to include it in our |
| 62 | + project. |
| 63 | + |
| 64 | +---------------------------------------------------------------- |
| 65 | +[mailing list is the primary place] |
| 66 | + |
| 67 | +One thing to note is that I do not respond to a pull request in private, |
| 68 | +and Github pull request, as I understand it, is very private in nature. |
| 69 | +The patches are to be reviewed on the main mailing list first. |
| 70 | + |
| 71 | +It's OK to say "these patches are also available in my repository at |
| 72 | +Github whose URL is this" in the commentary part of the final submission |
| 73 | +message after the list reaches consensus that your change is a good thing, |
| 74 | +and that may reduce the chance of mistakes when I accept the patches |
| 75 | +especially if the series is large, so I am not saying that repositories |
| 76 | +people have Github have no value to my workflow. But it will not come |
| 77 | +into the picture before the final submission phase. |
| 78 | + |
| 79 | +---------------------------------------------------------------- |
| 80 | +[going incremental after hitting next] |
| 81 | + |
| 82 | +Once a commit hits 'next', it gets improved only by piling incremental |
| 83 | +updates on top with explanation. The idea is: if all of us thought it |
| 84 | +has seen enough eyeballs and is good enough for 'next', yet we later |
| 85 | +find there was something we all missed, that is worth a separate |
| 86 | +explanation, e.g., "The primary motivation behind the series is still |
| 87 | +good, but for such and such reasons we missed this case we are |
| 88 | +fixing." |
| 89 | + |
| 90 | +Unless it turns out that the approach was fundamentally wrong and such |
| 91 | +an incremental update boils down to almost reverting the earlier one |
| 92 | +entirely and replacing it with the newer one. In such a case, we do |
| 93 | +revert the earlier and replace it with the newer, in 'next'. |
| 94 | + |
| 95 | +---------------------------------------------------------------- |
0 commit comments