Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Maybe if we can’t document a concrete solution, we can document or codify a procedure. Should for example the owner of the feature branch always work with the release manager when there are issues? On November 16, 2018 at 10:26:11, Michael Miklavcic ( michael.miklav...@gmail.com) wrote: It's a good suggestion, and I've been thinking about how to best handle this. Honestly, the right answer might be to do git rebase on master from the PR branch rather than a merge. That might avoid this situation altogether. Of course, that also comes with all the obligatory warnings about rebasing publicly shared branches, and that anyone how has their own copy will get conflicts. But I think that risk is probably minimal. I'll put something together that covers both options along with the problems and solutions for each. Hopefully that will make future collaboration that fits this pattern easier for people. On Fri, Nov 16, 2018, 5:42 AM Otto Fowler > Maybe this is worth a confluence entry, not as a guide, but just to > document what you did. > > On November 15, 2018 at 19:07:40, Michael Miklavcic ( > michael.miklav...@gmail.com) wrote: > > Ok, this is finally merged! Whew! > > Here's how I polished up the history at the end. I used other feature > branch merges as a guideline around commit messaging. > > * fcd644ca 2018-11-15 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (mmiklavc via mmiklavc) (HEAD -> > master, origin/master, origin/HEAD, master-merge) [mmiklavc] > |\ > | * 8bf3b6ec 2018-11-15 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (mmiklavc via mmiklavc) closes > apache/metron#1242 (stella-es-base2) [mmiklavc] > | * e7e19fbb 2018-10-08 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (cstella via mmiklavc) [cstella] > * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in > Management UI (sardell via nickwallen) closes apache/metron#1217 [sardell] > > On Thu, Nov 15, 2018 at 4:29 PM Michael Miklavcic < > michael.miklav...@gmail.com> wrote: > >> Absolutely, that's part of what I did to validate. This output below also >> exactly matches the diff I get when I run it from the raw PR branch. >> >> git diff master --stat >> Upgrading.md >> | 7 +++ >> dependencies_with_url.csv >> | 2 + >> metron-deployment/Kerberos-manual-setup.md >> | 154 >> ++--- >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml >> | 9 >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py >> | 2 - >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/params/params_linux.py >> | 3 +- >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/themes/metron_theme.json >> | 10 >> >> metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/MetaAlertServiceImpl.java >> | 2 +- >> metron-platform/elasticsearch-shaded/pom.xml >> | 47 >> +++- >> >> metron-platform/elasticsearch-shaded/src/main/resources/META-INF/log4j-provider.properties >> | 18 --- >> metron-platform/metron-common/README.md >> | 48 >> + >> metron-platform/metron-common/src/main/config/zookeeper/global.json >> | 1 - >> >> metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigOption.java >> | 7 +++ >> metron-platform/metron-elasticsearch/README.md >> | 45 >> +++- >> metron-platform/metron-elasticsearch/pom.xml >> | 32 >> +-- >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClient.java >> | 245 >> >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClientFactory.java >>| 189 >> + >> >> metron-platform/metron-elasticsearch/src/ma
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
It's a good suggestion, and I've been thinking about how to best handle this. Honestly, the right answer might be to do git rebase on master from the PR branch rather than a merge. That might avoid this situation altogether. Of course, that also comes with all the obligatory warnings about rebasing publicly shared branches, and that anyone how has their own copy will get conflicts. But I think that risk is probably minimal. I'll put something together that covers both options along with the problems and solutions for each. Hopefully that will make future collaboration that fits this pattern easier for people. On Fri, Nov 16, 2018, 5:42 AM Otto Fowler > Maybe this is worth a confluence entry, not as a guide, but just to > document what you did. > > On November 15, 2018 at 19:07:40, Michael Miklavcic ( > michael.miklav...@gmail.com) wrote: > > Ok, this is finally merged! Whew! > > Here's how I polished up the history at the end. I used other feature > branch merges as a guideline around commit messaging. > > * fcd644ca 2018-11-15 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (mmiklavc via mmiklavc) (HEAD -> > master, origin/master, origin/HEAD, master-merge) [mmiklavc] > |\ > | * 8bf3b6ec 2018-11-15 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (mmiklavc via mmiklavc) closes > apache/metron#1242 (stella-es-base2) [mmiklavc] > | * e7e19fbb 2018-10-08 | METRON-1834: Migrate Elasticsearch from > TransportClient to new Java REST API (cstella via mmiklavc) [cstella] > * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in > Management UI (sardell via nickwallen) closes apache/metron#1217 [sardell] > > On Thu, Nov 15, 2018 at 4:29 PM Michael Miklavcic < > michael.miklav...@gmail.com> wrote: > >> Absolutely, that's part of what I did to validate. This output below also >> exactly matches the diff I get when I run it from the raw PR branch. >> >> git diff master --stat >> Upgrading.md >> | 7 +++ >> dependencies_with_url.csv >> | 2 + >> metron-deployment/Kerberos-manual-setup.md >> | 154 >> ++--- >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml >> | 9 >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py >> | 2 - >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/params/params_linux.py >> | 3 +- >> >> metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/themes/metron_theme.json >> | 10 >> >> metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/MetaAlertServiceImpl.java >> | 2 +- >> metron-platform/elasticsearch-shaded/pom.xml >> | 47 >> +++- >> >> metron-platform/elasticsearch-shaded/src/main/resources/META-INF/log4j-provider.properties >> | 18 --- >> metron-platform/metron-common/README.md >> | 48 >> + >> metron-platform/metron-common/src/main/config/zookeeper/global.json >> | 1 - >> >> metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigOption.java >> | 7 +++ >> metron-platform/metron-elasticsearch/README.md >> | 45 >> +++- >> metron-platform/metron-elasticsearch/pom.xml >> | 32 >> +-- >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClient.java >> | 245 >> >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClientFactory.java >>| 189 >> + >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientConfig.java >> | 187 >> >> >> metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientOptio
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Maybe this is worth a confluence entry, not as a guide, but just to document what you did. On November 15, 2018 at 19:07:40, Michael Miklavcic ( michael.miklav...@gmail.com) wrote: Ok, this is finally merged! Whew! Here's how I polished up the history at the end. I used other feature branch merges as a guideline around commit messaging. * fcd644ca 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) (HEAD -> master, origin/master, origin/HEAD, master-merge) [mmiklavc] |\ | * 8bf3b6ec 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (stella-es-base2) [mmiklavc] | * e7e19fbb 2018-10-08 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (cstella via mmiklavc) [cstella] * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in Management UI (sardell via nickwallen) closes apache/metron#1217 [sardell] On Thu, Nov 15, 2018 at 4:29 PM Michael Miklavcic < michael.miklav...@gmail.com> wrote: > Absolutely, that's part of what I did to validate. This output below also > exactly matches the diff I get when I run it from the raw PR branch. > > git diff master --stat > Upgrading.md > | 7 +++ > dependencies_with_url.csv > | 2 + > metron-deployment/Kerberos-manual-setup.md > | 154 > ++--- > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml > | 9 > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py > | 2 - > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/params/params_linux.py > | 3 +- > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/themes/metron_theme.json > | 10 > > metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/MetaAlertServiceImpl.java > | 2 +- > metron-platform/elasticsearch-shaded/pom.xml > | 47 > +++- > > metron-platform/elasticsearch-shaded/src/main/resources/META-INF/log4j-provider.properties > | 18 --- > metron-platform/metron-common/README.md > | 48 > + > metron-platform/metron-common/src/main/config/zookeeper/global.json > | 1 - > > metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigOption.java > | 7 +++ > metron-platform/metron-elasticsearch/README.md > | 45 > +++- > metron-platform/metron-elasticsearch/pom.xml > | 32 > +-- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClient.java > | 245 > > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClientFactory.java >| 189 > + > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientConfig.java > | 187 > > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientOptions.java >| 60 + > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchColumnMetadataDao.java > | 101 ++- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java > | 21 > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java >| 2 +- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertSearchDao.java > | 6 ++- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Ok, this is finally merged! Whew! Here's how I polished up the history at the end. I used other feature branch merges as a guideline around commit messaging. * fcd644ca 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) (HEAD -> master, origin/master, origin/HEAD, master-merge) [mmiklavc] |\ | * 8bf3b6ec 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (stella-es-base2) [mmiklavc] | * e7e19fbb 2018-10-08 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (cstella via mmiklavc) [cstella] * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in Management UI (sardell via nickwallen) closes apache/metron#1217 [sardell] On Thu, Nov 15, 2018 at 4:29 PM Michael Miklavcic < michael.miklav...@gmail.com> wrote: > Absolutely, that's part of what I did to validate. This output below also > exactly matches the diff I get when I run it from the raw PR branch. > > git diff master --stat > Upgrading.md > | 7 +++ > dependencies_with_url.csv > | 2 + > metron-deployment/Kerberos-manual-setup.md > | 154 > ++--- > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml > | 9 > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py > | 2 - > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/params/params_linux.py > | 3 +- > > metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/themes/metron_theme.json > | 10 > > metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/MetaAlertServiceImpl.java > | 2 +- > metron-platform/elasticsearch-shaded/pom.xml > | 47 > +++- > > metron-platform/elasticsearch-shaded/src/main/resources/META-INF/log4j-provider.properties > | 18 --- > metron-platform/metron-common/README.md > | 48 > + > metron-platform/metron-common/src/main/config/zookeeper/global.json > | 1 - > > metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigOption.java > | 7 +++ > metron-platform/metron-elasticsearch/README.md > | 45 > +++- > metron-platform/metron-elasticsearch/pom.xml > | 32 > +-- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClient.java > | 245 > > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClientFactory.java >| 189 > + > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientConfig.java > | 187 > > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientOptions.java >| 60 + > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchColumnMetadataDao.java > | 101 ++- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java > | 21 > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java >| 2 +- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertSearchDao.java > | 6 ++- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertUpdateDao.java > | 4 +- > > metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/Elas
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Absolutely, that's part of what I did to validate. This output below also exactly matches the diff I get when I run it from the raw PR branch. git diff master --stat Upgrading.md | 7 +++ dependencies_with_url.csv | 2 + metron-deployment/Kerberos-manual-setup.md | 154 ++--- metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml | 9 metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/metron_service.py | 2 - metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/scripts/params/params_linux.py | 3 +- metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/themes/metron_theme.json | 10 metron-interface/metron-rest/src/main/java/org/apache/metron/rest/service/impl/MetaAlertServiceImpl.java | 2 +- metron-platform/elasticsearch-shaded/pom.xml | 47 +++- metron-platform/elasticsearch-shaded/src/main/resources/META-INF/log4j-provider.properties | 18 --- metron-platform/metron-common/README.md | 48 + metron-platform/metron-common/src/main/config/zookeeper/global.json | 1 - metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/ConfigOption.java | 7 +++ metron-platform/metron-elasticsearch/README.md | 45 +++- metron-platform/metron-elasticsearch/pom.xml | 32 +-- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClient.java | 245 metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/client/ElasticsearchClientFactory.java | 189 + metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientConfig.java | 187 metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/config/ElasticsearchClientOptions.java | 60 + metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchColumnMetadataDao.java | 101 ++- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java | 21 metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java | 2 +- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertSearchDao.java | 6 ++- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertUpdateDao.java | 4 +- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchRequestSubmitter.java | 13 ++--- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchRetrieveLatestDao.java | 28 +- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchSearchDao.java | 19 ++- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchUpdateDao.java | 19 +++ metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/ElasticsearchUtils.java | 182 +++--- metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/FieldMapping.java | 32 +++ metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/utils/FieldProperties.java | 36 + metron-platform/metron-elasticse
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Can you diff the trees to be sure? On November 15, 2018 at 17:52:40, Michael Miklavcic ( michael.miklav...@gmail.com) wrote: So amazingly, this still has results in conflicts, but I am able to resolve them manually in a sensible fashion. git merge -X theirs es-rebased 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" So where I landed gives a history like the following: * df1195aa 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (HEAD -> master-merge) [mmiklavc] |\ | * 590b3669 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (stella-es-base2) [mmiklavc] | * a7c7dc28 2018-10-08 | Casey Stella - elasticsearch rest client migration base work (stella-es-base) [cstella] * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in Management UI (sardell via nickwallen) closes apache/metron#1217 (origin/master, origin/HEAD, master) [sardell] ... I can modify a7c7dc28 commit message as well and hopefully this will be good for everyone? Cheers, Mike On Thu, Nov 15, 2018 at 2:29 PM Justin Leet 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 > 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 mast
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
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 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 > 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
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
So amazingly, this still has results in conflicts, but I am able to resolve them manually in a sensible fashion. git merge -X theirs es-rebased 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" So where I landed gives a history like the following: * df1195aa 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (HEAD -> master-merge) [mmiklavc] |\ | * 590b3669 2018-11-15 | METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API (mmiklavc via mmiklavc) closes apache/metron#1242 (stella-es-base2) [mmiklavc] | * a7c7dc28 2018-10-08 | Casey Stella - elasticsearch rest client migration base work (stella-es-base) [cstella] * | 0c4c622b 2018-11-14 | METRON-1749 Update Angular to latest release in Management UI (sardell via nickwallen) closes apache/metron#1217 (origin/master, origin/HEAD, master) [sardell] ... I can modify a7c7dc28 commit message as well and hopefully this will be good for everyone? Cheers, Mike On Thu, Nov 15, 2018 at 2:29 PM Justin Leet 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 > 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 an
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
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 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#12
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
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 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-inter
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
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 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 com
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Yes, definitely.
On Thu, Nov 15, 2018 at 2:01 PM Casey Stella 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.singleto
Re: [DISCUSS] Attribution and merging the Elasticsearch client migration
Can you at least rename your commits to have METRON-1834 prefixing them?
On Thu, Nov 15, 2018 at 15:19 Michael Miklavcic
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 fieldTypes =
> dao.getColumnMetadata(Collections.singletonList("snort"));
> <<< HEAD
> Assert.assertEquals(32, fieldTypes.size());
> Assert.assertEquals(FieldType.KEYWORD,
> fieldTypes.get("sig_generator"));
> ===
> Assert.assert
[DISCUSS] Attribution and merging the Elasticsearch client migration
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 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)
