With the bundles, archetype, plug-in and parser moves gone 777 will be much smaller though.
Also I did not mean to admit Mike’s doc reviews, sorry. On September 20, 2017 at 18:04:20, Otto Fowler ([email protected]) wrote: > I am not taking any of this as starting to do a flame war :) And your > concern is about review not merging right? If it is all reviewed and > accepted, why would 6 merges be preferable? That doesn’t make sense. > Breaking it up only makes sense with regards to review, so that is how > I’ll take it. > > And I *am* all ears for how to break up the code. Like literally HOW. > Like : ‘I would diff a folder with master, create a new branch for the pr > and patch just that’. > Just 'break up the massive working thing into smaller chucks' is not > enough for my relative experience with git and github to you Nick. > > So if all we want are code review chunks, regardless of being able to run > them and see them work then ( but also not break the regular build and > full_dev ): > > 1. PR for Bundle-Lib And Maven Plugin ( but that is already reviewed ) > code review only, doesn’t do anything > 2. PR for Archetype ( just building the result, unit integration tests ) > code review and generate project review. projects built by archetype : > => not buildable not installable not testable > relies on changes to parser testing etc. to work with non-hard coded paths > for test resources etc etc > 3. PR with Parsers copied to extensions > code review only. not buildable not installable not testable > 4. PR with the rest of 777 + grok fixes ( all non ui non rest ) -> move to > new parser structure, load install, grok support etc etc. > regression testable, status quo ante functionality. > still huge > will still be a problem > cannot be broken down and built / pass travis at this point > this is the integration > 5. PR of Rest > can install and uninstall via rest > 6. PR of UI > can list, install uninstall via UI > > I did not think then that we wanted PRs that don’t build or test or run > the changes they have ( but the branch builds and passes travis ), but just > have code to review. > > But, as you said, it has been since April, and only Bundles has been > reviewed. If nobody but Matt is even going to _attempt_ code review, then > it may now be implicitly required that I do this. > > > On September 20, 2017 at 16:41:46, Nick Allen ([email protected]) wrote: > > I was just responding to your statement "Well, if there is an alternative > merge strategy, I’m all ears." I understand that you disagree, but this is > an alternative strategy. I think this approach is feasible and I outlined > how here [1]. > > The criticism of this approach is the amount of effort and time to do > this, which I agree with. It will take a good deal of effort. > I acquiesced to this criticism at the time because I thought we were trying > to push this change in quickly. But it has been a couple months now and we > still have a bunch of code to merge. > > This is no criticism of you or anyone here. What Otto has been doing in > keeping this branch merged with master is god damn heroic. And I am so > excited to see this functionality hit master. I just wanted to brainstorm > on ideas to help get this in. Right now, I am unsure of the path and > timeline on how we are going to get this merged. > > Please read this in the kindest possible light. I am not trying to start > a flame war or offend anyone. > > > [1] https://github.com/apache/metron/pull/530#issuecomment-306806217 > > > > On Wed, Sep 20, 2017 at 3:37 PM Otto Fowler <[email protected]> > wrote: > >> “Bite off small chunks” is what I keep hearing. I have no idea how to do >> that from an already integrated and working branch. >> Do you mean I… >> - create patch files of whole directories and do a pr per directory, but >> the build doesn’t work? >> >> >> On September 20, 2017 at 15:07:56, Nick Allen ([email protected]) wrote: >> >> > Otto: Well, if there is an alternative merge strategy, I’m all ears. >> >> Yes, the alternative strategy is what I mentioned. :) Copied in below. >> >> > Nick: Are you going to bite-off small chunks of the feature branch and >> introduce PRs against master? >> >> >> >> >> >> On Wed, Sep 20, 2017 at 12:02 PM Otto Fowler <[email protected]> >> wrote: >> >>> Well, if there is an alternative merge strategy, I’m all ears. >>> As it stands, the only complete code / conceptual review has been the >>> bundles-lib by mattf. >>> So there is still a great deal of review activity to do. >>> >>> Also conceptually there is ( might not be complete ) >>> >>> * the discuss thread topic from this morning ( metron parsers v. >>> extensions wrt registration and management ) >>> * default configurations ( parser, enrichment, indexing, elasticsearch) >>> >>> >>> >>> >>> On September 20, 2017 at 11:37:17, Nick Allen ([email protected]) >>> wrote: >>> >>> Ok, so it sounds like you are envisioning a "big bang" merge between the >>> feature branch and master at some point. Not ideal in my mind, but maybe >>> we need to be more pragmatic with this one. >>> >>> Do we have other tasks (like these PRs) to complete before we are ready >>> to consider the merge? >>> >>> I am just looking to help however I can in killing this feature branch; >>> meaning I want your code in master, as soon as possible. :) >>> >>> >>> >>> >>> >>> >>> On Wed, Sep 20, 2017 at 10:27 AM Otto Fowler <[email protected]> >>> wrote: >>> >>>> So, were the functionality is not FB related, I have been doing PR’s >>>> against master ( such as hdfs service ability to set permissions ). >>>> I don’t think we have talked about the end game PR from feature to >>>> master, I am don’t know how we would do multiple PRs to bring it down >>>> once ‘accepted’. I think that approach needs to to be discussed and >>>> agreed on. >>>> >>>> >>>> On September 20, 2017 at 10:23:44, Nick Allen ([email protected]) >>>> wrote: >>>> >>>> So it sounds like these PRs do move us closer to bringing the two >>>> branches together. But I think I am missing your high-level approach >>>> though. >>>> >>>> How are we going to get all of the functionality from the feature >>>> branch into master? Are you going to bite-off small chunks of the feature >>>> branch and introduce PRs against master? >>>> >>>> >>>> >>>> On Wed, Sep 20, 2017 at 9:23 AM Otto Fowler <[email protected]> >>>> wrote: >>>> >>>>> Right now, these two PR’s are stacked, the UI depends on the >>>>> BundleSystem changes. >>>>> I would rather bring them down to feature before I do master >>>>> integration, than integrate master >>>>> into feature and then bring it out to the stacked PR’s. Simple as >>>>> that. >>>>> >>>>> >>>>> >>>>> On September 20, 2017 at 08:35:09, Nick Allen ([email protected]) >>>>> wrote: >>>>> >>>>> Hi Otto - >>>>> >>>>> What is the plan for bringing the feature branch and master together? >>>>> Do >>>>> these PRs move us closer to bringing the two branches together? >>>>> >>>>> Thanks >>>>> >>>>> >>>>> >>>>> On Wed, Sep 20, 2017 at 8:19 AM Otto Fowler <[email protected]> >>>>> wrote: >>>>> >>>>> > Can I get a bump on https://github.com/apache/metron/pull/747 and >>>>> > https://github.com/apache/metron/pull/761? >>>>> > >>>>> > The next time I take master is going to be a bit of integration >>>>> work, and I >>>>> > would like to unstack and get my stuff integrated first. >>>>> > >>>>> >>>>>
