Totally agree Otto. The other concern here is that I believe the automated release scripts need to see the Jira's on the commits. See my latest update as I think I got something workable.
On Thu, Nov 15, 2018 at 3:30 PM Otto Fowler <ottobackwa...@gmail.com> wrote: > 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) > > >> > > > >> > > > > > >
