HI, Can we continue the review process with the next PR in the line? https://github.com/apache/metron/pull/1414
First three PR get merged to the feature branch by Ryan. Thanks for that! On Tue, Jun 11, 2019 at 10:20 PM Michael Miklavcic < michael.miklav...@gmail.com> wrote: > Agreed Ryan, I know this is a bit of a cart before the horse scenario. > > This isn't the most desirable of circumstances, and we're doing our best to > accommodate the contribution because it's a good feature to have, and the > work so far looks pretty solid. We have a bit more flexibility in a feature > branch than we would if this was (as it was originally) submitted against > master - I think the most important thing here is we do > architecture/code/test review on an individual PR basis. They've split up > this work pretty fine-grained, so I don't think it should be too hard for > us to work through. When we get through PRs 1-14 from a code review > perspective, we'll have to do a run of manual acceptance tests on the final > product before accepting the feature branch into master. Anything that > appears to be broken or missing will simply need another PR to address it, > and we can get it in. I actually don't think this should be too bad. Again, > thank you to Shane, Tibor, and Tamas for breaking this down. Just be > careful in the future when you're collaborating on something of this size > that you either A) split off the work in fully functioning PRs, e.g. > refactoring work or library changes can very easily be submitted against > master without breaking anything or B) start with a feature branch and > discuss thread that lays out your plan for delivering the feature in that > FB. > > Please review Apache's guide on consensus building, as well - > http://community.apache.org/committers/consensusBuilding.html > > On Tue, Jun 11, 2019 at 12:01 PM Ryan Merriman <merrim...@gmail.com> > wrote: > > > "We planning to add the changes to the latest PR as additional commits to > > avoid impacting the PR sequence. We will refer to the source PR in the > > commit message of the fix. Also adding a link to the comment section of > the > > source PR of the change request to the fixing commit to make them > > connected." > > > > I don't think this is going to work. If changes are requested and > applied > > in a different PR, how would you test the original PR? What would you do > > if there were merge conflicts introduced between the current and final > PR? > > What's the point of even having any PRs before the final one if they > won't > > get committed or changed? I don't see any way to avoid having to > propagate > > changes through all subsequent PRs. > > > > On Wed, May 29, 2019 at 7:03 AM Tibor Meller <tibor.mel...@gmail.com> > > wrote: > > > > > Hi all, > > > > > > *We still need some volunteer reviewers for this feature.* All > individual > > > PR is under 1000 line of changes except one and it is due to an > > > autogenerated package.lock.json file. > > > Just a heads up: parser aggregation by default turned on for bro, snort > > and > > > yarn parser on full dev. Without this changeset, full dev is broken. > > > > > > > > > https://lists.apache.org/thread.html/beeb4cfddfca7958a22ab926f72f52f46a33c42edce714112df9a2da@%3Cdev.metron.apache.org%3E > > > > > > > > > > > > On Fri, May 24, 2019 at 3:20 PM Tibor Meller <tibor.mel...@gmail.com> > > > wrote: > > > > > > > Please find below the list of the PRs we opened for Parser > Aggregation. > > > > With Shane, Tamas we tried to provide as much information as possible > > to > > > > make the reviewing process easier. > > > > Please keep that in mind these PRs are not against muster but a > Parser > > > > Aggregation feature branch. > > > > If you like to read more about the process we followed with these PRs > > > > please read the previous three message in this thread. > > > > > > > > PR#1 METRON-2114: [UI] Moving components to sensor parser module > > > > <https://github.com/apache/metron/pull/1410> > > > > PR#2 METRON-2116: [UI] Removing redundant AppConfigService > > > > <https://github.com/apache/metron/pull/1411> > > > > PR#3 METRON-2117: [UI] Aligning models to grouping feature > > > > <https://github.com/apache/metron/pull/1412> > > > > PR#4 METRON-2115: [UI] Aligning UI to the parser aggregation AP > > > > <https://github.com/apache/metron/pull/1414> > > > > PR#5 METRON-2122: [UI] Fixing early app config access issue > > > > <https://github.com/apache/metron/pull/1415> > > > > PR#6 METRON-2124: [UI] Move status information and start/stop to the > > > > Aggregate level <https://github.com/apache/metron/pull/1418> > > > > PR#7 METRON-2125: [UI] Making changes visible in the parser list by > > > > marking changed items <https://github.com/apache/metron/pull/1422> > > > > PR#8 METRON-2131: Add NgRx and related dependencies > > > > <https://github.com/apache/metron/pull/1423> > > > > PR#9 METRON-2133: Add NgRx effects to communicate with the server > > > > <https://github.com/apache/metron/pull/1424> > > > > PR#10 METRON-2134: Add NgRx reducers to perform parser and group > > changes > > > > in the store <https://github.com/apache/metron/pull/1425> > > > > PR#11 METRON-2135: Add NgRx actions to trigger state changes > > > > <https://github.com/apache/metron/pull/1426> > > > > PR#12 METRON-2136: Add parser aggregation sidebar > > > > <https://github.com/apache/metron/pull/1427> > > > > PR#13 METRON-2137: Implement drag and drop mechanism and wire NgRx > > > > <https://github.com/apache/metron/pull/1428> > > > > PR#14 METRON-2138: Code clean up > > > > <https://github.com/apache/metron/pull/1429> > > > > PR#15 METRON-2139: Refactoring sensor-parser-config.component and > wire > > > > NgRx <https://github.com/apache/metron/pull/1430> > > > > > > > > Thanks, > > > > Tibor > > > > > > > > > > > > On Thu, May 23, 2019 at 11:45 AM Tibor Meller < > tibor.mel...@gmail.com> > > > > wrote: > > > > > > > >> Yes, am expecting that some change request will rase due to the > > review. > > > >> We planning to add the changes to the latest PR as additional > commits > > to > > > >> avoid impacting the PR sequence. We will refer to the source PR in > the > > > >> commit message of the fix. Also adding a link to the comment section > > of > > > the > > > >> source PR of the change request to the fixing commit to make them > > > connected. > > > >> > > > >> On Wed, May 22, 2019 at 5:49 PM Michael Miklavcic < > > > >> michael.miklav...@gmail.com> wrote: > > > >> > > > >>> Tibor, that sounds reasonable to me. If PR #1 ends up requiring > code > > > >>> changes, will you guys just percolate those up through the > remaining > > k > > > >>> PRs > > > >>> in order, or just the final PR? I'm wondering how this works in > > > reference > > > >>> to your last point in #5 about rebasing. > > > >>> > > > >>> On Wed, May 22, 2019 at 8:47 AM Tibor Meller < > tibor.mel...@gmail.com > > > > > > >>> wrote: > > > >>> > > > >>> > I would like to describe quickly *our approach to breaking down > > > Parser > > > >>> > Aggregation PR for smaller chunks* > > > >>> > > > > >>> > *1. we squashed the commits in the original development branch* > > > >>> > - when we started to open smaller PRs from the commits from the > > > >>> original > > > >>> > branch, we found ourself opening PRs out of historical states of > > the > > > >>> code > > > >>> > instead of the final one > > > >>> > - none of those states of development are worth (or make sense) > to > > be > > > >>> > reviewed (initial phases of development are included in the > > original > > > >>> commit > > > >>> > history, multiple iterations of refactoring, etc.) > > > >>> > - while the actual development history was irrelevant, the > > > attribution > > > >>> > aspect of it was still important > > > >>> > *2. we divided** the changes by the original authors* > > > >>> > - the original contributors were sardell, ruffle1986 and tiborm > > > >>> > - we isolated the changes that belong to each individual > > contributor > > > >>> > *3. each of us identified smaller but belonging changesets * > > > >>> > - with this, we ended up opening 5 PRs from tiborm, 3 from > sardell > > > and > > > >>> 6 > > > >>> > from ruffle1986 > > > >>> > - each of these are smaller than 500 lines of changes, which > makes > > > the > > > >>> task > > > >>> > of reviewing easier > > > >>> > - they have their own context and purpose described by the PR and > > the > > > >>> > related Jira ticket > > > >>> > *4. Each PR introduces a single new commit which is meant to be > > > >>> reviewed* > > > >>> > - with this we were able to open PRs on top of each others work, > > but > > > >>> the > > > >>> > reviewer is still able to identify what changes were introduced > and > > > >>> > described by the pr simply by focusing on the last commit > > > >>> > - the commit introduced by the PR has the same commit message as > > the > > > >>> title > > > >>> > of the PR to make it easier to find > > > >>> > *5. Only the last PR is meant to be merged into the feature > branch* > > > >>> > - the last PR also introduces a single new commit to being > reviewed > > > >>> > - this contains all the commits from the previous PRs that belong > > to > > > >>> parser > > > >>> > aggregation > > > >>> > - it builds fine in Travis > > > >>> > - it's fully functional and ready to being tested against full > dev > > > >>> > - If we only merge the last PR, we don't have to rebase and > > recreate > > > >>> all of > > > >>> > our PRs due to merge conflicts that will result from to > conflicting > > > >>> > histories (which is common in feature branch work) > > > >>> > > > > >>> > Once all the Pull Requests are open, I will submit a list of all > of > > > >>> them to > > > >>> > this discussion thread. > > > >>> > > > > >>> > On Wed, May 8, 2019 at 3:58 PM Otto Fowler < > > ottobackwa...@gmail.com> > > > >>> > wrote: > > > >>> > > > > >>> > > You need to be a committer, that is all I think. > > > >>> > > I would not use the github UI for it though, I do it through > the > > > cli > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> > > On May 8, 2019 at 09:45:24, Michael Miklavcic ( > > > >>> > michael.miklav...@gmail.com > > > >>> > > ) > > > >>> > > wrote: > > > >>> > > > > > >>> > > Not that I'm aware of. Nick and Otto, you've created them > before, > > > >>> did you > > > >>> > > need any special perms? > > > >>> > > > > > >>> > > On Wed, May 8, 2019 at 3:57 AM Shane Ardell < > > > >>> shane.m.ard...@gmail.com> > > > >>> > > wrote: > > > >>> > > > > > >>> > > > This morning, we started to break down our work as Michael > > > >>> suggested in > > > >>> > > > this thread. However, it looks like I don't have permission > to > > > >>> create a > > > >>> > > new > > > >>> > > > branch in the GitHub UI or push a new branch to the > > apache/metron > > > >>> repo. > > > >>> > > Is > > > >>> > > > this action restricted to PMC members only? > > > >>> > > > > > > >>> > > > Shane > > > >>> > > > > > > >>> > > > On Wed, May 8, 2019 at 9:06 AM Tamás Fodor < > > > ftamas.m...@gmail.com> > > > >>> > > wrote: > > > >>> > > > > > > >>> > > > > Here's the process we've gone through in order to implement > > the > > > >>> > > feature. > > > >>> > > > > > > > >>> > > > > At the beginning we had some bootstrap work like creating a > > > mock > > > >>> API > > > >>> > > > > (written in NodeJS) because we were a few steps ahead the > > > backend > > > >>> > part. > > > >>> > > > But > > > >>> > > > > this is in a totally different repository so it doesn't > > really > > > >>> count. > > > >>> > > We > > > >>> > > > > also had to wire NgRX, our chosen 3rd party that supports > the > > > >>> flux > > > >>> > flow > > > >>> > > > to > > > >>> > > > > get a better state management. When we were ready to kick > off > > > >>> > > > implementing > > > >>> > > > > the business logic in, we splited up the work by > subfeatures > > > like > > > >>> > drag > > > >>> > > > and > > > >>> > > > > dropping table rows. At this point, we created a POC > without > > > NgRX > > > >>> > just > > > >>> > > to > > > >>> > > > > let you have the feeling of how it works in real life. > Later > > > on, > > > >>> > after > > > >>> > > > > introducing NgRX, we had to refactor it a little bit > > obviously > > > to > > > >>> > make > > > >>> > > it > > > >>> > > > > compatible with NgRX. There were other subfeatures like > > > creating > > > >>> and > > > >>> > > > > editing groups in a floating pane on the right side of the > > > >>> window. > > > >>> > > > > When the real backend API was ready we made the necessary > > > >>> changes and > > > >>> > > > > tested whether it worked how it was expected. There were a > > few > > > >>> > > difference > > > >>> > > > > between how we originally planned the API and the current > > > >>> > > implementation > > > >>> > > > so > > > >>> > > > > we had to adapt it accordingly. While we were implementing > > the > > > >>> > > features, > > > >>> > > > we > > > >>> > > > > wrote the unit tests simultaneously. The latest task on the > > > >>> feature > > > >>> > was > > > >>> > > > > restricting the user from aggregating parsers together. > > > >>> > > > > > > > >>> > > > > As a first iteration, we've decided to put the restriction > in > > > >>> because > > > >>> > > it > > > >>> > > > > requires a bigger effort on the backend to deal with that. > In > > > my > > > >>> > > opinion, > > > >>> > > > > we should get rid of the restriction because it's not > > intuitive > > > >>> and > > > >>> > > very > > > >>> > > > > inconvenient. In my opinion, we should let the users to > > > >>> aggregate the > > > >>> > > > > running parsers together and do the job to handle this edge > > > case > > > >>> on > > > >>> > the > > > >>> > > > > backend accordingly. > > > >>> > > > > > > > >>> > > > > What do you think, guys? > > > >>> > > > > > > > >>> > > > > Hope this helps. > > > >>> > > > > > > > >>> > > > > Tamas > > > >>> > > > > > > > >>> > > > > On Tue, May 7, 2019 at 4:34 PM Michael Miklavcic < > > > >>> > > > > michael.miklav...@gmail.com> wrote: > > > >>> > > > > > > > >>> > > > > > This was my expectation as well. > > > >>> > > > > > > > > >>> > > > > > Shane, Tibor, Tamas - how did you go about breaking this > > down > > > >>> into > > > >>> > > > chunks > > > >>> > > > > > and/or microchunks when you collaborated offline? As Nick > > > >>> > mentioned, > > > >>> > > > you > > > >>> > > > > > obviously split up work and shared it amongst yourselves. > > > Some > > > >>> > > > > explanation > > > >>> > > > > > around this process would be helpful for reviewers as > well. > > > We > > > >>> > might > > > >>> > > be > > > >>> > > > > > able to provide better guidance and examples to future > > > >>> contributors > > > >>> > > as > > > >>> > > > > > well. > > > >>> > > > > > > > > >>> > > > > > I talked a little bit with Shane about this offline last > > > week. > > > >>> It > > > >>> > > looks > > > >>> > > > > > like you guys effectively ran a local feature branch. > > > >>> Replicating > > > >>> > > that > > > >>> > > > > > process in a feature branch in Apache is probably what > you > > > guys > > > >>> > > should > > > >>> > > > > > be doing for a change this size. We don't have hard > limits > > on > > > >>> line > > > >>> > > > change > > > >>> > > > > > size, but in the past it's been somewhere around 2k-3k > > lines > > > >>> and > > > >>> > > above > > > >>> > > > > > being the tipping point for discussing a feature branch. > > > >>> Strictly > > > >>> > > > > speaking, > > > >>> > > > > > line quantity alone is not the only metric, but it's > > relevant > > > >>> here. > > > >>> > > If > > > >>> > > > > you > > > >>> > > > > > want to make smaller incremental changes locally, there's > > > >>> nothing > > > >>> > to > > > >>> > > > keep > > > >>> > > > > > you from doing that - I would only advise that you > consider > > > >>> > squashing > > > >>> > > > > those > > > >>> > > > > > commits (just ask if you're unclear about how to handle > > that) > > > >>> into > > > >>> > a > > > >>> > > > > single > > > >>> > > > > > larger commit/chunk when you're ready to publish them as > a > > > >>> chunk to > > > >>> > > the > > > >>> > > > > > public feature branch. So it would look something like > > this: > > > >>> > > > > > > > > >>> > > > > > Commits by person locally > > > >>> > > > > > Shane: 1,2,3 -> squash as A > > > >>> > > > > > Tibor: 4,5,6 -> squash as B > > > >>> > > > > > Tamas: 7,8,9 -> squash as C > > > >>> > > > > > > > > >>> > > > > > Commits by person in Apache > > > >>> > > > > > Shane: A > > > >>> > > > > > Tibor: B > > > >>> > > > > > Tamas: C > > > >>> > > > > > > > > >>> > > > > > We need to maintain a record of attribution. Your real > > > >>> workflow may > > > >>> > > not > > > >>> > > > > be > > > >>> > > > > > that cleanly delineated, but you can choose how you want > to > > > >>> squash > > > >>> > in > > > >>> > > > > those > > > >>> > > > > > cases. Even in public collaboration, there are plenty of > > > cases > > > >>> > where > > > >>> > > > > folks > > > >>> > > > > > submit PRs against PRs, abstain from accepting > attribution, > > > >>> and it > > > >>> > > all > > > >>> > > > > gets > > > >>> > > > > > squashed down into one person's final PR commit. There > are > > > many > > > >>> > > > options. > > > >>> > > > > > > > > >>> > > > > > Hope this helps. > > > >>> > > > > > > > > >>> > > > > > Best, > > > >>> > > > > > Mike > > > >>> > > > > > > > > >>> > > > > > On Mon, May 6, 2019 at 8:19 AM Nick Allen < > > > n...@nickallen.org> > > > >>> > > wrote: > > > >>> > > > > > > > > >>> > > > > > > Have you considered creating a feature branch for the > > > effort? > > > >>> > This > > > >>> > > > > would > > > >>> > > > > > > allow you to break the effort into chunks, where the > > result > > > >>> of > > > >>> > each > > > >>> > > > PR > > > >>> > > > > > may > > > >>> > > > > > > not be a fully working "master-ready" result. > > > >>> > > > > > > > > > >>> > > > > > > I am sure you guys tackled the work in chunks when > > > >>> developing it, > > > >>> > > so > > > >>> > > > > > > consider just replaying those chunks onto the feature > > > branch > > > >>> as > > > >>> > > > > separate > > > >>> > > > > > > PRs. > > > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > On Mon, May 6, 2019 at 5:24 AM Tibor Meller < > > > >>> > > tibor.mel...@gmail.com> > > > >>> > > > > > >>> > > > > > > wrote: > > > >>> > > > > > > > > > >>> > > > > > > > I wondered on the weekend how we could split that PR > to > > > >>> smaller > > > >>> > > > > chunks. > > > >>> > > > > > > > That PR is a result of almost 2 months of development > > > and I > > > >>> > don't > > > >>> > > > see > > > >>> > > > > > how > > > >>> > > > > > > > to split that to multiple WORKING parts. It is as it > > is a > > > >>> whole > > > >>> > > > > working > > > >>> > > > > > > > feature. If we split it by packages or files we could > > > >>> provide > > > >>> > > > smaller > > > >>> > > > > > > > non-functional PR's, but can end up having a broken > > > >>> Management > > > >>> > UI > > > >>> > > > > after > > > >>> > > > > > > > having the 1st PR part merged into master. I don't > > think > > > >>> that > > > >>> > > would > > > >>> > > > > be > > > >>> > > > > > > > acceptable by the community (or even by me) so I > would > > > >>> like to > > > >>> > > > > suggest > > > >>> > > > > > > two > > > >>> > > > > > > > other option to help review PR#1360. > > > >>> > > > > > > > > > > >>> > > > > > > > #1 We could extend that PR with our own author > comments > > > in > > > >>> > > Github. > > > >>> > > > > That > > > >>> > > > > > > > would help following which code part belongs to where > > and > > > >>> why > > > >>> > it > > > >>> > > > was > > > >>> > > > > > > > necessary. > > > >>> > > > > > > > #2 We can schedule an interactive code walkthrough > call > > > >>> with > > > >>> > the > > > >>> > > > ones > > > >>> > > > > > who > > > >>> > > > > > > > interested in reviewing or the particular changeset. > > > >>> > > > > > > > > > > >>> > > > > > > > Please share your thoughts on this! Which version > would > > > >>> support > > > >>> > > you > > > >>> > > > > the > > > >>> > > > > > > > best? Or if you have any other idea let us know. > > > >>> > > > > > > > > > > >>> > > > > > > > PS: I think the size of our PR's depends on how small > > > >>> > > independently > > > >>> > > > > > > > deliverable changesets we can identify before we > > starting > > > >>> to > > > >>> > > > > implement > > > >>> > > > > > a > > > >>> > > > > > > > relatively big new feature. Unfortunately, we missed > to > > > do > > > >>> that > > > >>> > > > with > > > >>> > > > > > this > > > >>> > > > > > > > feature. > > > >>> > > > > > > > > > > >>> > > > > > > > On Fri, May 3, 2019 at 1:49 PM Shane Ardell < > > > >>> > > > > shane.m.ard...@gmail.com> > > > >>> > > > > > > > wrote: > > > >>> > > > > > > > > > > >>> > > > > > > > > NgRx was only used for the aggregation feature and > > > >>> doesn't go > > > >>> > > > > beyond > > > >>> > > > > > > > that. > > > >>> > > > > > > > > I think the way I worded that sentence may have > > caused > > > >>> > > > confusion. I > > > >>> > > > > > > just > > > >>> > > > > > > > > meant we use it to manage more pieces of state > within > > > the > > > >>> > > > > aggregation > > > >>> > > > > > > > > feature than just previous and current state of > > grouped > > > >>> > > parsers. > > > >>> > > > > > > > > > > > >>> > > > > > > > > On Fri, May 3, 2019 at 1:32 AM Michael Miklavcic < > > > >>> > > > > > > > > michael.miklav...@gmail.com> wrote: > > > >>> > > > > > > > > > > > >>> > > > > > > > > > Shane, thanks for putting this together. The > > updates > > > >>> on the > > > >>> > > > Jira > > > >>> > > > > > are > > > >>> > > > > > > > > useful > > > >>> > > > > > > > > > as well. > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > (we used it for more than just that in this > > > feature, > > > >>> but > > > >>> > > that > > > >>> > > > > was > > > >>> > > > > > > the > > > >>> > > > > > > > > > initial reasoning) > > > >>> > > > > > > > > > What are you using NgRx for in the submitted work > > > that > > > >>> goes > > > >>> > > > > beyond > > > >>> > > > > > > the > > > >>> > > > > > > > > > aggregation feature? > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > On Thu, May 2, 2019 at 12:22 PM Shane Ardell < > > > >>> > > > > > > shane.m.ard...@gmail.com > > > >>> > > > > > > > > > > > >>> > > > > > > > > > wrote: > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > Hello everyone, > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > In response to discussions in the 0.7.1 release > > > >>> thread, I > > > >>> > > > > wanted > > > >>> > > > > > to > > > >>> > > > > > > > > > start a > > > >>> > > > > > > > > > > thread regarding the parser aggregation work > for > > > the > > > >>> > > > Management > > > >>> > > > > > UI. > > > >>> > > > > > > > For > > > >>> > > > > > > > > > > anyone who has not already read and tested the > PR > > > >>> > locally, > > > >>> > > > I've > > > >>> > > > > > > > added a > > > >>> > > > > > > > > > > detailed description of what we did and why to > > the > > > >>> JIRA > > > >>> > > > ticket > > > >>> > > > > > > here: > > > >>> > > > > > > > > > > > > https://issues.apache.org/jira/browse/METRON-1856 > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > I'm wondering what the community thinks about > > what > > > >>> we've > > > >>> > > > built > > > >>> > > > > > thus > > > >>> > > > > > > > > far. > > > >>> > > > > > > > > > Do > > > >>> > > > > > > > > > > you see anything missing that must be part of > > this > > > >>> new > > > >>> > > > feature > > > >>> > > > > in > > > >>> > > > > > > the > > > >>> > > > > > > > > UI? > > > >>> > > > > > > > > > > Are there any strong objections to how we > > > >>> implemented it? > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > I’m also looking to see if anyone has any > > thoughts > > > >>> on how > > > >>> > > we > > > >>> > > > > can > > > >>> > > > > > > > > possibly > > > >>> > > > > > > > > > > simplify this PR. Right now it's pretty big, > and > > > >>> there > > > >>> > are > > > >>> > > a > > > >>> > > > > lot > > > >>> > > > > > of > > > >>> > > > > > > > > > commits > > > >>> > > > > > > > > > > to parse through, but I'm not sure how we could > > > break > > > >>> > this > > > >>> > > > work > > > >>> > > > > > out > > > >>> > > > > > > > > into > > > >>> > > > > > > > > > > separate, smaller PRs opened against master. We > > > >>> could try > > > >>> > > to > > > >>> > > > > > > > > cherry-pick > > > >>> > > > > > > > > > > the commits into smaller PRs and then merge > them > > > >>> into a > > > >>> > > > feature > > > >>> > > > > > > > branch, > > > >>> > > > > > > > > > but > > > >>> > > > > > > > > > > I'm not sure if that's worth the effort since > > that > > > >>> will > > > >>> > > only > > > >>> > > > > > reduce > > > >>> > > > > > > > the > > > >>> > > > > > > > > > > number commits to review, not the lines > changed. > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > As an aside, I also want to give a little > > > background > > > >>> into > > > >>> > > the > > > >>> > > > > > > > > > introduction > > > >>> > > > > > > > > > > of NgRx in this PR. To give a little background > > on > > > >>> why we > > > >>> > > > chose > > > >>> > > > > > to > > > >>> > > > > > > do > > > >>> > > > > > > > > > this, > > > >>> > > > > > > > > > > you can refer to the discussion thread here: > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > > >>> > > > > >>> > > > > > > https://lists.apache.org/thread.html/06a59ea42e8d9a9dea5f90aab4011e44434555f8b7f3cf21297c7c87@%3Cdev.metron.apache.org%3E > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > We previously discussed introducing a better > way > > to > > > >>> > manage > > > >>> > > > > > > > application > > > >>> > > > > > > > > > > state in both UIs in that thread. It was > decided > > > that > > > >>> > NgRx > > > >>> > > > was > > > >>> > > > > a > > > >>> > > > > > > > great > > > >>> > > > > > > > > > tool > > > >>> > > > > > > > > > > for many reasons, one of them being that we can > > > >>> piecemeal > > > >>> > > it > > > >>> > > > > into > > > >>> > > > > > > the > > > >>> > > > > > > > > > > application rather than doing a huge rewrite of > > all > > > >>> the > > > >>> > > > > > application > > > >>> > > > > > > > > state > > > >>> > > > > > > > > > > at once. The contributors in this PR (myself > > > >>> included) > > > >>> > > > decided > > > >>> > > > > > this > > > >>> > > > > > > > > would > > > >>> > > > > > > > > > > be a perfect opportunity to introduce NgRx into > > the > > > >>> > > > Management > > > >>> > > > > UI > > > >>> > > > > > > > since > > > >>> > > > > > > > > > we > > > >>> > > > > > > > > > > need to manage the previous and current state > > with > > > >>> the > > > >>> > > > grouping > > > >>> > > > > > > > feature > > > >>> > > > > > > > > > so > > > >>> > > > > > > > > > > that users can undo the changes they've made > (we > > > >>> used it > > > >>> > > for > > > >>> > > > > more > > > >>> > > > > > > > than > > > >>> > > > > > > > > > just > > > >>> > > > > > > > > > > that in this feature, but that was the initial > > > >>> > reasoning). > > > >>> > > In > > > >>> > > > > > > > addition, > > > >>> > > > > > > > > > we > > > >>> > > > > > > > > > > greatly benefited from this when it came time > to > > > >>> debug > > > >>> > our > > > >>> > > > work > > > >>> > > > > > in > > > >>> > > > > > > > the > > > >>> > > > > > > > > UI > > > >>> > > > > > > > > > > (the discussion in the above thread link goes a > > > >>> little > > > >>> > more > > > >>> > > > > into > > > >>> > > > > > > the > > > >>> > > > > > > > > > > advantages of debugging with NgRx and > DevTools). > > > >>> Removing > > > >>> > > > NgRx > > > >>> > > > > > from > > > >>> > > > > > > > > this > > > >>> > > > > > > > > > > work would reduce the numbers of lines changed > > > >>> slightly, > > > >>> > > but > > > >>> > > > it > > > >>> > > > > > > would > > > >>> > > > > > > > > > still > > > >>> > > > > > > > > > > be a big PR and a lot of that code would just > > move > > > >>> to the > > > >>> > > > > > component > > > >>> > > > > > > > or > > > >>> > > > > > > > > > > service level in the Angular application. > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > Shane > > > >>> > > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> > > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >> > > > > > >
