kinow commented on pull request #154:
URL: https://github.com/apache/commons-vfs/pull/154#issuecomment-774442260


   >If that makes it easier for you, OK for me. Here's part 1: #158
   >As soon as you merge it, I'll rebase this PR and drop the merged commits.
   
   Thanks @MaxKellermann !
   
   >In projects I maintain, if I like some obvious-good commits, I cherry-pick 
them and then start discussing the others, and I expect the submitter to rebase 
the PR with only the unmerged commits remaining.
   
   :+1: 
   
   >What I certainly would never do is fold all PR commits into one large 
commit like you did with #155 - 3 commits were folded into one (9c4302e). That 
loses a lot of context, makes the resulting commit hard to 
read/verify/understand, and makes git-bisect very hard. That's your project, 
your maintenance decision, so I'm not going to argue, just mentioning it 
because that's one thing that would make project management hard for me.
   
   My workflow is normally to check out the branch locally, rebase onto master 
— or make sure it's rebased — and then checkout master and merge with 
`--no-ff`. If the PR has many commits, I may squash them if they appear to be 
draft/WIP commits, or ask the user if s/he can squash it (asked last week I 
think, in Apache Commons Imaging).
   
   But sometimes we may squash without thinking/asking. Some times when I 
create a PR I mention that I haven't squashed for some specific reason, and 
tell the maintainers I'd be happy to squash it if they think it's necessary. If 
you mention that I'm sure the final reviewer will consider it too.
   
   Some projects also have a hard-policy to always merge a single commit 
(Apache OpenNLP for example, though not sure if that changed).
   
   Finally, I forgot to say **thank you** for the great commit messages. They 
are short, and explain what's included in that commit :+1: 


----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to