Proper attribution and the correct code are the most important things, not
the number of commits.


On November 15, 2018 at 16:29:04, Justin Leet (justinjl...@gmail.com) wrote:

I took a look at this with Mike a bit, and it seems like it's pretty
painful and without a clear way to avoid remerging conflicts. If the
latest attempt doesn't work, I'm in favor of getting it in and just getting
it down to as few commits as reasonably possible.

On Thu, Nov 15, 2018 at 4:12 PM Michael Miklavcic <
michael.miklav...@gmail.com> wrote:

> I'm attempting 1 more option, which would be to do a "git merge
> --strategy-option theirs" after having done the commit wrangling in the
PR
> branch. Will reply back with results.
>
> On Thu, Nov 15, 2018 at 2:02 PM Michael Miklavcic <
> michael.miklav...@gmail.com> wrote:
>
> > Yes, definitely.
> >
> > On Thu, Nov 15, 2018 at 2:01 PM Casey Stella <ceste...@gmail.com>
wrote:
> >
> >> Can you at least rename your commits to have METRON-1834 prefixing
them?
> >> On Thu, Nov 15, 2018 at 15:19 Michael Miklavcic <
> >> michael.miklav...@gmail.com>
> >> wrote:
> >>
> >> > https://github.com/apache/metron/pull/1242
> >> >
> >> > TL;DR
> >> > I'd like to discuss the best option to merge METRON-1834 into
master.
> I
> >> > want to propose handling this like a feature branch and merging it
> >> as-is.
> >> > ---
> >> >
> >> > I'm sure most folks' initial reaction will be some skepticism akin
to
> >> "have
> >> > you tried turning it off again," as this was my initial reaction as
> >> well.
> >> > It does not seem like this should be difficult. And I'm hoping that
> this
> >> > may be some esoteric thing on my system, though I believe this is a
> real
> >> > problem. A rather tedious explanation follows of what I've tried and
> the
> >> > problems encountered along the way. What seemed like a really simple
> >> > problem instead appears to be a bit much for Git to handle without
> >> > requiring redoing merges and another full round of testing. I'd much
> >> prefer
> >> > to avoid that in this instance.
> >> >
> >> > This PR is ready to be merged into master. It's recent and very
close
> to
> >> > fully up to date in the branch. Latest master merges cleanly. There
is
> >> an
> >> > attribution to Casey Stella for the base point of this PR that I
need
> to
> >> > include when getting this into master. When I created my branch, I
> >> > collapsed his initial set of commits into a single squashed commit
on
> >> > master at the time, and I started to work from there. Over time, I
> made
> >> a
> >> > number of additional commits and merges from master. Now for the
> issues.
> >> >
> >> > Originally, my expectation was that I could have 2 commits - the
> >> original
> >> > squashed commit from Casey along with all my additional commits (and
> the
> >> > merges with master) right on top. Nice clean history on master.
Turns
> >> out,
> >> > this doesn't work as cleanly as expected because a combination of
the
> >> > multiple merges and the need to keep the original commit with
> >> attribution
> >> > to Casey's work. A normal git pull --squash works fine, as expected,
> >> but we
> >> > lose the base commit, and therefore the requisite attribution. Here
> are
> >> > some other things I've tried, to no avail.
> >> >
> >> > 1. Git pull --squash after a merge with master. This will squash
> the
> >> > entire tree back to the branch point. No good.
> >> > 2. Git rebase -i master. Allows you to cleanly apply changes, but
> >> then
> >> > it ends up having problems with a clean rebase and shows
> conflicts. I
> >> > expect this is because of the merge history being necessary.
> >> > 3. Checking out a branch from the base point squashed commit from
> >> Casey,
> >> > and attempt to apply my changes on top. Numerous methods for
> >> > squashing/rebasing my changes on top applies nicely in the branch.
> >> But
> >> > then
> >> > it once again causes merge conflicts when I attempt to get this
> onto
> >> > master. Things I attempted include: manually copying files,
> rebasing
> >> > all my
> >> > commits plus merges on top of the base commit, git merge --squash,
> >> > intimidation.
> >> >
> >> > For one example of the result I'm talking about, this looks "good"
but
> >> it's
> >> > missing a ton of recent commits because they get caught up in the
> rebase
> >> > and get squashed in with my commit. When you attempt to merge this
> onto
> >> > master, it is just plain wrong (see example below with merge
> conflicts).
> >> > * 22c3b3bc 2018-11-15 | METRON-1834: Migrate Elasticsearch from
> >> > TransportClient to new Java REST API (mmiklavc via mmiklavc) closes
> >> > apache/metron#1242 (HEAD -> stella-es-base2) [mmiklavc]
> >> > * 84232e90 2018-10-08 | METRON-1834: Elasticsearch rest client
> migration
> >> > base work starting point for apache/metron#1242 (cstella via
mmiklavc)
> >> > [cstella]
> >> > * 5bfc08c5 2018-10-08 | METRON-1792 Simplify Profile Definitions in
> >> > Integration Tests (nickwallen) closes apache/metron#1211
[nickwallen]
> >> >
> >> > Here's 1 merge conflict (say what??)
> >> > CONFLICT (rename/rename): Rename
> >> >
> >> >
> >>
>
"metron-interface/metron-config/src/app/rxjs-operators.ts"->"metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ParserRunnerResults.java"

> >> > in branch "HEAD" rename
> >> >
> >> >
> >>
>
"metron-interface/metron-config/src/app/rxjs-operators.ts"->"metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/FieldMapping.java"

> >> > in "stella-es-base2"
> >> >
> >> > If I attempt to use rebase on master instead of merge, it really
seems
> >> to
> >> > mess up the files. Again, another example where I have TODO's that
are
> >> most
> >> > definitely removed by a commit in my branch and also do not exist in
> >> > master. I'm not sure what's happening here, but I don't trust it.
> >> > {
> >> > //TODO: It shouldn't require an assertEventually() here as it
> >> should
> >> > be synchronous.
> >> > // Before merging, please figure out why.
> >> > TestUtils.assertEventually(() -> Assert.assertEquals(14,
> >> > dao.getColumnMetadata(Collections.singletonList("snort")).size()));
> >> > Map<String, FieldType> fieldTypes =
> >> > dao.getColumnMetadata(Collections.singletonList("snort"));
> >> > <<<<<<< HEAD
> >> > Assert.assertEquals(32, fieldTypes.size());
> >> > Assert.assertEquals(FieldType.KEYWORD,
> >> > fieldTypes.get("sig_generator"));
> >> > =======
> >> > Assert.assertEquals(FieldType.INTEGER,
> >> > fieldTypes.get("snort_field"));
> >> > >>>>>>> METRON-1834: Elasticsearch rest client migration base work
> >> starting
> >> > point for apache/metron#1242 (cstella via mmiklavc)
> >> >
> >>
> >
>

Reply via email to