[Development] Android: Merge or no merge?

Oswald Buddenhagen oswald.buddenhagen at digia.com
Thu Feb 21 14:16:19 CET 2013


On Mon, Feb 11, 2013 at 05:15:45PM +0100, Eskil Abrahamsen Blomfeldt wrote:
> We have a disagreement on how to integrate the Android port into Qt 
> which we need to get resolved.
> 
> Here's the discussion:
> 
>      https://codereview.qt-project.org/#change,47480
> 
> tl;dr: Should we merge or rebase the Android branch when integrating it 
> into Qt mainline?
> [...]

> 1. The initial commit does not have a review-stamp.
>
that's not the problem. that's not even *a* problem.
the problem is that this commit would never have gotten an approval -
for very good reasons.

> 2. There is clutter in the commit log, i.e. changes that change code 
> style and whitespace errors.
> 
yes, they do. big time. but that's not even the problem.
the problem are the dozens of workarounds and other hacks, that were
later amended, sometimes multiple times, and finally reverted.
this mess will stay in the history. it doesn't matter how much you
optimize "blame" - every other way of browsing the history
(specifically, my favorite (because very efficient) git log -p) is
messed up.

> In my opinion, the amount of history we submit seems like a luxury
> problem.
>
no. it is a problem of longer-term workflow optimization.

> I would very much like to be able to track the development of the
> Android branch after merging it, so in my opinion having the history
> intact is nice, even if they are not interesting to everyone.
>
i postulate that this proposition is wrong.
it's simply not true that "more history is better".
the main information you want to get out of the history is why something
was done in a particular way. somewhat secondary is the question when
something came in. for neither of these questions the actual history is
relevant, if what ends up in the mainline satisfies the requirements of
the commit policy: atomicity and good commit messages, including
documenting non-obvious decisions.
everything else is just noise.

> One of the major history-based problems in the Qt 4 days was that we
> would squash together commits from topic branches and lose the history
> in those branches.
> 
yes, this is a terrible practice, indeed.
fortunately, this is not what re-shaping the history would be about.

> Squashing the "clutter" commits and rewriting the entire history is a 
> lot of work,
>
yes, it is.
however, it is not *that* much work.

> and I fear it will jeopardize our chances of meeting the Qt 5.1
> deadline.
>
ah, yeah, there it is: we have a deadline to meet. let's ignore good
practice!

> A compromise would be to rebase on top of dev, review the initial
> commit, and then rebase the rest of the branch on top of this. This
> would break the references in Jira tasks, though, so personally I'd
> prefer not to do this.
>
i postulate that this is mostly irrelevant:
- it is possible to backup the branch, say refs/dead-heads/android. this
  way all external references would stay valid, even if these commits
  were not part of the regular git history.
  note also that the commits are kept alive in gerrit by all the reviews
  which were done on top of them.
- many of the jira tasks refer to issues with the initial code drop.
  iow, these tasks are merely part of the review process, and have no
  long-term value. they shouldn't have existed in the first place -
  under normal circumstances it would have all happened on gerrit.
- for "real" tasks, it would be possible to automatically fix up the
  jira references to the rebased sha1s.

> I also wonder how it's achieved technically,
>
with a forced direct push.
i just did that to the winrt branch upon request. it was a remarkably
painless experience.

a reasonably efficient process which would lead to a clean history
(which includes a proper review of every commit on its own merits,
rather than improving from the original code drop) would look like that:
- first squash all fixup commits and reverts which were done in the
  branch
  - it is possible to record the change-ids of the fixups in this commit
    to make absolutely sure that the "real" history can be easily
    tracked. however, for many (if not most) of these commits this
    doesn't really make sense.
- then rebase on top of dev. this eliminates a bunch of commits and
  shrinks others.
- chop up the initial commit into atomic parts: general portability
  improvements and generalizations, buildsystem enhancements, then the
  addition of the java module and the specific porting work.
- submit the extracted groundwork directly for dev, as eskil has already
  done to some degree. this gets the full incremental review and
  CI-testing, and ensures that we get no diverging solutions for the
  same problems (cf., the concurrent ios port).
- rebase again. what is left now can be seen as a "proper" long-lived
  branch and can be actually merged, and possibly even continued *after*
  it has been merged the first time.

i'll also point out that the winrt and ios branches are both following a
history re-shaping approach. these ports are arguably smaller, and they
have the big difference that they are not starting from an initial code
drop. however: the contribution of the android support to qt creator was
already done the right way: extraction of the groundwork, and later the
submission of the main contribution, with dozens of review iterations.
just as it should be.

regards



More information about the Development mailing list