Le 20/01/2023 à 15:03, Aaron Hill a écrit :
On 2023-01-20 3:22 am, Jean Abou Samra wrote:Rewriting history is just something that one might not be used to coming from other projects, but perfectly fine for our purposes.It was impressed upon me to treat rewriting history as fraught with peril, potentially even Bad(tm) in the Ghostbusters sense:"Try to imagine all life as you know it stopping instantaneously andevery molecule in your body exploding at the speed of light." -- Egon SpenglerOne might argue that anything in your development branch is not really part of "history" yet as it has not been accepted. (The situation gets murkier when people are forking forks.) As such, it *should* be safe to mess about with commits to clean up typos or catch missing files, what have you. And I would agree this results in a cleaner submission that is easier to review for correctness. (I probably should clarify my earlier comments that I do not intentionally make my commits messy, just that I usually do not stress about them being so absolutely pristine; again, I am used to work being squashed, so any niceness I put in there gets lost.)
Indeed, overwriting history is Very Bad™ for shared branches, but not really a problem for development branches as long as a single developer edits them.
So I can see --amend being useful for the little things. But let's say during a review, it is determined that the scope could increase to cover more items than originally planned but that still feels part of the same submission. (Anything too distinct really should be an independent request.) Now, you might not necessarily want to force all of the new development work into the existing commit. Reviewers might even appreciate seeing the individual slices of the task more cleanly delineated. In a sense, there is some "history" to the process that might be worth preserving during this stage.
Yes, exactly! And that is what our process is designed to facilitate :) If there are two commits in the MR Commit A: add XYZ Commit B: take advantage of XYZ to better reimplement QRSTand you want to modify XYZ to address reviewers' suggestions, the more "usual"
workflow is adding a new commit on top, yielding Commit A: add XYZ Commit B: take advantage of XYZ to better reimplement QRST Commit C: fix typo in XYZ whereas in our process, you "rebase -i" to get Commit A: add XYZ [amended version with a different commit ID]Commit B: take advantage of XYZ to better reimplement QRST [also a new commit ID]
and * the next reviewer who looks at the MR sees only the logical structure, not cluttered by the fixes, * everything can be cleanly integrated into master as-is, without squashing A and B (which you would need if you wanted C not to be separate from A, due to the ordering). IOW: it may seem counterintuitive, but our workflow that involves editing commits in-place instead of adding more commits is precisely aimed at allowing more structure between the commits, by making the history of the branch reflect what you eventually want to end up in the repo, a *logical* structure, rather than the temporal structure driven by "I just happened to fix this before that". Not that I am an out-and-out fan of this workflow, but its tradeoffs have served us well so far, I would say. While this may not be the most usual workflow with GitHub/GitLab, it's not unheard of at all. E.g., the more traditional email-based workflow (in Git itself, the Linux kernel, many GNU projects, etc.) has the same logical separation of commits where you amend patches even after making more patches on top of them. https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#separate-your-changes
OpenPGP_signature
Description: OpenPGP digital signature
