Omit On September 20, 2017 at 19:20:27, Otto Fowler ([email protected]) wrote:
> 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. >>>>>> > >>>>>> >>>>>>
