28

I've always agreed with Mercurial's mantra 1, however, now that Mercurial comes bundled with the rebase extension and it is a popular practice in git, I'm wondering if it could really be regarded as a "bad practice", or at least bad enough to avoid using. In any case, I'm aware of rebasing being dangerous after pushing.

OTOH, I see the point of trying to package 5 commits in a single one to make it look niftier (specially at in a production branch), however, personally I think would be better to be able to see partial commits to a feature where some experimentation is done, even if it is not as nifty, but seeing something like "Tried to do it way X but it is not as optimal as Y after all, doing it Z taking Y as base" would IMHO have good value to those studying the codebase and follow the developers train of thought.

My very opinionated (as in dumb, visceral, biased) point of view is that programmers like rebase to hide mistakes... and I don't think this is good for the project at all.

So my question is: have you really found valuable to have such "organic commits" (i.e. untampered history) in practice?, or conversely, do you prefer to run into nifty well-packed commits and disregard the programmers' experimentation process?; whichever one you chose, why does that work for you? (having other team members to keep history, or alternatively, rebasing it).


1 per Google DVCS analysis, in Mercurial "History is Sacred".

gnat
  • 21,442
  • 29
  • 112
  • 288
dukeofgaming
  • 13,943
  • 6
  • 50
  • 77
  • could you provide reference to where it is stated as "Mercurial's mantra"? – gnat Jun 05 '12 at 06:09
  • Now that you mention it, I think it actually comes from Google's DVCS analysis http://code.google.com/p/support/wiki/DVCSAnalysis (but the fact remains) – dukeofgaming Jun 05 '12 at 14:41
  • possible duplicate of [When should the VCS history of a project be deleted?](http://programmers.stackexchange.com/questions/181780/when-should-the-vcs-history-of-a-project-be-deleted) – gnat Sep 03 '13 at 08:44

6 Answers6

24

The History is sacred, the Present is not. You can split your DVCS "tree" in two parts:

  • The past/history which contains an accurate view of how you have reached the current state of the code. This part of the history grow over time

  • The present which part you are currently working on to make you code evolve. This tip most part of the history have about always the same size.

Every code you released or used somehow is part of the past. The past is sacred because you need to be able to reproduce a setup or understand what introduced a regression. You shall never ever rewrite the past. In git you usually never rewrite anything once it is in master: master is the past part of the history. In Mercurial you have this public phase concept that keep track of the past part of your "tree" and enforce its immutability.

The present part of the code are the changeset you are currently working on. The feature branch that you are trying to make usable, bugfree and properly refactored. It is perfectly fine to rewrite it it is even a good idea because it make the past part more pretty, simple and usable. Mercurial track this in the draft phase.

So yes, please rebase if it improves your history. Mercurial will prevent you to shoot yourself in the foot if you are rebasing stuff you should not.

Marmoute
  • 376
  • 1
  • 3
7

Not all mistakes are of the kind that you need to hide- for example, I once accidentally committed a binary file to my repo. Tampering with history is only bad when the fault is not exclusively in the history itself, like files committed which shouldn't be.

DeadMG
  • 36,794
  • 8
  • 70
  • 139
7

Code review is a lot easier when there's one big cohesive change as opposed to a lot of small dependent ones.

When I work on a new feature, I like to commit lots of little changes in my branch. When I'm ready to submit the patch, I collapse those little commits into one big commit that represents all of the new code required for that feature. This is where rebase comes in handy.

On the other hand, rebase is ill-advised if the commits have nothing to do with each other. Multiple feature additions should be separate commits (and ideally come from separate branches).

chrisaycock
  • 6,655
  • 3
  • 30
  • 54
  • Code-review is a good reason to rebase. And with the protection Mercurial phases provide I think I can sleep at night (http://mercurial.selenic.com/wiki/Phases) – dukeofgaming Jun 05 '12 at 14:44
  • 4
    there are plenty of tools that make code review of multiple related changes fairly trivial, so on its own, I don't buy this as an answer – jk. Jun 05 '12 at 21:48
  • 4
    How is there any difference between reviewing the difference between the base revision and feature complete revision, and reviewing the single commit of that re-based set of commits? It is going to look identical if the re-base did it's job correctly! – Mark Booth Jun 06 '12 at 14:19
  • 3
    What you describe here goes straight against the [advice in the Mercurial wiki](http://mercurial.selenic.com/wiki/OneChangePerPatch). Small "obviously correct" changesets are preferred for reviews. Collapsing a feature branch into a single changeset is not normal in Mercurial — I've seen it recommended much more often in Git where `git rebase -i` lets you do this easily and selectively. The nearest Mercurial equivalent is [histedit](http://mercurial.selenic.com/wiki/HisteditExtension). – Martin Geisler Jun 07 '12 at 22:02
  • 9
    I completely disagree. The last couple of years we used a tool for code reviews, and there was nothing I hated more getting one large 30+ file changeset submitted for review. It is a lot easier to get a lot of small changes. E.g. I renamed this class to xyz because it better reflects the modified responsibility. I added method nn because I will be needing it for blah. Much easier to handle smaller reviews. – Pete Jun 08 '12 at 18:05
  • 2
    It is far better to preserve history, as there is nothing preventing you getting a single diff across a large number of commits. Example for Git: http://stackoverflow.com/questions/3338126/git-how-to-diff-the-same-file-between-two-different-commits-on-the-same-branch Example for Mercurial: http://stackoverflow.com/questions/2760810/mercurial-creating-a-diff-of-two-commits – Dan Lyons Jun 08 '12 at 19:08
  • @DanLyons I'm curious how you found this question and answer, given that both were posted several days ago. I don't mind that you downvoted something you disagree with, but I do find it odd that you, Pete, and Martin Geisler each cast your first-ever downvote against this in the span of 24 hours. Was this answer linked somewhere? – chrisaycock Jun 08 '12 at 20:21
  • 3
    I've heard this idea quite a bit in the git community of collapsing lots of commits down into one before you push to the official repo, but this has never been helpful to me. I'd much rather have the small commits with meaningful messages. As others have noted, you can do a diff across multiple changesets. – Chris Sutton Jun 09 '12 at 21:42
  • I find code reviews easier when the changes are small and incremental. (And especially when diff-heavy changes like re-indenting a file or renaming a popular method are kept separate from the "action" changes.) If a branch has some doing-and-undoing and I want to see the global difference that the branch will bring, I do a fake, temporary merge back to default and look at the diffs. – Joshua Goldberg Sep 28 '16 at 03:31
5

Your question describes history as a set of ordered code changes, and asks whether or not organic commits clue future readers into the development process. Yet as a release / integration engineer, I don't often think of history as code. I'm more engrossed with the story my history tells, the institutional knowledge it retains, and whether or not it allows me to quickly debug problems.

I don't think organic workflows do this well, even my own. Qualities I value about DVCS when I'm coding -- cheap branches, quick commits, saves to the remote early and often -- are not what I value as my company's integration manager. I issue git rebase, git merge, git diff, and git apply far more often in that role than git add or git commit. Tools like rebase allow me to transform the code I'm given from something that functions into something that can be maintained.

Illogical or vague commits aren't useful, but they are sinfully easy to write organically, when the primary concern is making the code work, not distributing it to others. Commits like Case 15: Fixed a problem, or Refactored <cranky legacy feature> make my maintenance-self cringe, even when I author them. None, however, induce the blackout rage like "incremental" commits. Consider this topic branch a developer handed me the other day:

$ git log production..topic --oneline
f9a1184 incremental update
156d299 incremental commits
e8d50b0 new incremental updates
511c18c incremental updates, refactor
1b46217 incremental upgrade
9e2b3b8 incremental update
fa68a87 incremental update

These things are Evil. It's like DVCS designed for Dr. Faustus. I'll give you quick and easy source control. You give me is your code maintainer's soul. All lazy commits are selfish acts. Many of us write them, but we also owe our future selves a logical, replicable, debuggable history. We can't do that without some way to rebase.

As for failed experiments, why not describe them in our (newly pristine) commit messages? A year from now I don't need a half-finished snippet. I just want a record of the aborted attempt, and perhaps a short explanation if I think I'm foolish enough to try it again. There are a lot of successful workflows in the world, but I'm struggling to think of any reason I'd knowingly commit broken code to a production codebase.

Christopher
  • 2,826
  • 1
  • 13
  • 8
2

Nothing should be sacred, but make sure you have good reasons for what you're doing. Rebasing is extremely powerful when used properly, but as with any powerful tool it can be confusing and cause problems if used carelessly.

Personally I find it very useful to rebase a local feature branch against trunk (or main development branch) before running the final tests and merging the feature into the main branch. That way I get to deal with any merge conflicts etc before actually merging.

harald
  • 1,953
  • 13
  • 16
1

IMHO, experiments usually belong to shelves or temporary branches, not baselines. If you follow this, there shouldn't be an issue, as all commits will be logically sound and add some value.

Coder
  • 6,958
  • 5
  • 37
  • 49
  • What you are saying is that you favor working branches over editing a baseline branch (i.e. master/default, production/release, vX.X, etc.)? – dukeofgaming Jun 04 '12 at 21:38