Ok, thanks for the clarification. On Fri, May 3, 2019 at 5:49 AM 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 > > > > > >
