[Development] Android: Merge or no merge?

[Knoll Lars]

Hi,

I'm with Eskil here.

The fact that the original import doesn't have a reviewed-by line on it is
a bit unfortunate, but it doesn't validate rewriting things and breaking
links to Jira. These can be important later on to find out why something
was done the way it's done.

I'm not a fan of squashing commits, let's rather keep the history intact.
It reflects what happened and how we reached the code we now have.

I also don't mind a bit of clutter in the commit log. Changes that clean
up coding style are good, and we'll always have these in some places.
Rebasing a huge branch to clean this up and push the cleanups into the
initial commit is a pretty large amount of work for a relatively small
benefit. I'd rather you spend your time on fixing bugs :)

So let's go ahead and merge the branch.

Cheers,
Lars

On 2/11/13 5:15 PM, "Eskil Abrahamsen Blomfeldt"
<eskil.abrahamsen-blomfeldt at digia.com> wrote:

>Hi,
>
>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.or,g/#change,47480
><https://codereview.qt-project.org/#change,47480>
>
>tl;dr: Should we merge or rebase the Android branch when integrating it
>into Qt mainline?
>
>The main issue originates from the fact that Necessitas was integrated
>as one large, atomic commit.
>
>Our plan to minimize the effect of this was to
>
>1. Create separate patches for the non-Android-specific parts of that
>change, and commit these separately to dev ahead of time. (In progress)
>2. Collect feedback on the total diff of the branch using Paul's large
>squashed commit and fix the issues as they are brought up. (In progress)
>3. Do a merge which retains the blame-history of the changes that were
>already committed to dev, so that the only parts which will show up as
>the large initial commit in a git blame are the lines which are
>Android-specific and which have not been disputed and therefore altered.
>
>This way we keep the SHA1s which are already linked to tasks in Jira and
>we have the full history of the Android port at least back to the point
>where Necessitas was integrated in Qt 5 (kind of like the initial Qt 5
>commit, but for the Android port.)
>
>However, there was some disagreement about whether this was acceptable
>or whether the branch should be rebased on top of dev instead.
>
>The problems:
>
>1. The initial commit does not have a review-stamp. The only way to get
>around this would be to rebase and rewrite the commit to include my
>review, or to post the commit itself to dev ahead of time and then merge
>on top of that, which would break the rule of only committing finished
>features.
>2. There is clutter in the commit log, i.e. changes that change code
>style and whitespace errors.
>
>In my opinion, the amount of history we submit seems like a luxury
>problem. 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. 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.
>
>Squashing the "clutter" commits and rewriting the entire history is a
>lot of work, and I fear it will jeopardize our chances of meeting the Qt
>5.1 deadline. Since it only serves to remove information and history and
>would take up too much valuable time, it does not seem justifiable to
>me. Since we did get reviews for all changes except the initial one, the
>idea was that we would not have to do this kind of history rewrite later.
>
>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 also wonder how it's achieved technically,
>since most of the changes have been reviewed already and therefore do
>not need additional reviews, but I'm sure this is not a huge problem.
>
>So what do you think?
>
>-- Eskil
>_______________________________________________
>Development mailing list
>Development at qt-project.org
>http://lists.qt-project.org/mailman/listinfo/development