gemmellr commented on pull request #684:
URL: https://github.com/apache/qpid-dispatch/pull/684#issuecomment-666318547


   I find myself thinking of just deleting this instead of posting as it turned 
out stupidly long, but meh, typed it so...
   
   I get the authorship bit to an extent, though if simply removing/undoing a 
bunch of stuff from the prior not-yet-pushed PR commit in the next ones I dont 
think the original really needs to go in the main repo in full as its arguably 
cluttering history at that point. Not all changes need to be accepted. E.g you 
could squash explicit removal bits and attribute the net added change to the 
original person if you arent overly caring that 'these are my changes undoing 
yours'. Or you could just ask them to undo the changes you think arent required.
   
   Either way the full commits could still have been here to reference without 
need to complicate history on the master (you could even reference the PR 
having intermediate history in a squashing commit), which is what I think files 
with commits of  'adds blah' and then immediately 'removes blah' effect in the 
same push does. 
   
   E.g to the earlier comment and also your mention of git blame, in this case 
the git blame will in some cases now be showing you essentially undoing stuff 
that effectively never existed on master before now, instead of just showing 
no-change or just what was net added change vs master/past-releases from before 
this PR. To understand the changes actually made to these parts later based on 
the blame you might now need to examine at least 2 or possible more updates 
from separate commits 'for the same change', which might end up explaining that 
'nothing much happened here except lots of churn', vs it being direct in the 
blame showing what did or didnt happen as a unit.
   
   To the description side of things, commit messages can have multiple lines 
and files can always be commented if its really necessary, plus JIRAs and PRs 
relating to the given commit(s) can also have explanatory detail in comments, 
which is a good part of why they exist and are referenced.
   
   I do think having several commits toward an end can make a lot of sense in 
some cases, e.g when its complicated feature or fix including refactors etc 
composed of several largely independent changes that sit well on their own 
(often to the extent theyll get their own JIRAs) rather than bunched together. 
This just doesnt feel like one of those times to me, and so instead having the 
various intermediate commits for whats mostly a single effective change just 
makes it harder to see what the change actually is later, requiring looking at 
the ensire set.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to