Stamatis>Personally, I always perform rebase followed by a forced push I was inclined to use that policy in early days, yet I think it should not be the main way.
Bellow assumes GitHub. If we happen to use Gerrit things might shine with a different colour. I suggest the following. FAQ: Q: I want to rebase/squash to make the PR shiny. Should I? A: No. It would complicate Q: I'm afraid all those oops/fixup commits will clutter git history. Should I rebase? A: No. Rebase/squash can be performed by committer if there's no other issues Q: Travis CI failed, but the failure is not caused by my changes (e.g. failed to download from Maven). Should I force-push to re-trigger the CI? A: No. Please create empty commit (git commit --allow-empty) and push it. Q: My PR is quite old, and I am afraid it is no longer valid. Should I rebase it? A: Yes. Rules for contributor: R1) Use feature branch when creating PR. Do not use yours master branch for PR. R2) Consider squashing the commits into meaningful ones before you create the PR. Do not expect "oops/fix/fixup" commits to land to Calcite master. R3) Feel free to force-push and squash commits during the first 10 minutes of PR lifetime R4) If PR was created more than 10 minutes ago, refrain from force-push R5) Do not force-push in case there's a pending discussion (in the PR and/or in JIRA) regarding the changes. Pending is vague, so I would suggest tp consider the discussion to be in pending state if the latest comment is within 2 weeks R6) Consider using appropriate commit message for the first commit in series. Consider duplicating the message to JIRA/PR, so it gets clear what is the nature of the change R7) Consider rebasing the PR on top of master if there are lots of new commits there Rules for committer/reviewer: R8) Consider squashing the commits manually rather than asking PR author to do that. If "commit is not squashed" is the sole comment, then both author and reviewer would have to spend time on one more review iteration with just a mechanical changes. Note: committer cannot just use "squash and merge" button in the GitHub UI Reasoning 1) Prefer non-rebase push, prefer regular commits on top of previously existing ones. It does make it easier to review. Review is async in its nature, and having a commit (or multiple of them) with new changes enables to review the changes later. "rebase + squash" makes it very complicated to review, especially if the diff is very small. On top of that, if new commits are just added, then reviewer can just point which of the variations is better. 2) I suppose "squash everything in single commit" can be performed by committer assuming the first commit has meaningful message. Squash is trivial, however crafting a message takes some time. 3) Sometimes it makes sense to squash the PR into several commits (there might be several fixes that relate to the same JIRA ticket), and I suggest that to be made after there's a consensus in general, and after all the other bits are resolved. 4) If the PR gets very old, it might make sense to rebase it on top of current master. That might be very valid point to squash commits. 5) Adding a dummy commit is the only option to re-launch Travis CI tests. Making dummy commit is way better than force-pushing all the changes with just different commit date. Vladimir
