iMHO since you are now a commuter you have the power to call it 5,16.0. And even make a release when you are ready.
On Thu, Nov 29, 2018 at 7:53 AM Jamie G. <jamie.goody...@gmail.com> wrote: > Hi Gary, > > Just want to clarify, you're asking to wait for 5.16.0 to be released, > than bring in the refactoring effort? > > If you feel 5.16.0 is imminent than I'm happy to hold off. I'd prefer > to get this in sooner rather than later so as to reduce the amount of > rebasing/cherry picking i need to do in the meantime. > > By the way, thank you for the support and looking at the code. I > really appreciate it :) > > Cheers, > Jamie > On Thu, Nov 29, 2018 at 8:56 AM Gary Tully <gary.tu...@gmail.com> wrote: > > > > Hey Arthur, > > I am not asserting that they need to be small. > > I am pointing out that they currently are small changes; there has > > been no significant refactor to date; it is all very conservative. > > Release 5.16.0 as a line in the sand, then move code around to make it > > better etc.. go for it. > > > > I know too well it is not perfect and I think it is great that there > > is interest in making it better. > > > > On Wed, 28 Nov 2018 at 16:22, Arthur Naseef <a...@amlinv.com> wrote: > > > > > > Hey Gary - I agree that these changes belong on a minor version > increase. > > > > > > What I don't understand is the assertion that the changes between > 5.15.x > > > and 5.16.0 need to be small. Given that the minor version bump can > mean > > > significant changes, as long as they are backward compatible, I see no > > > reason to adhere to a small set of changes between 5.15.x and 5.16.0. > Add > > > to that fact that ActiveMQ's minor releases are sometimes really major > > > changes (i.e. include breaking changes), and that makes even less > sense. > > > > > > Is there something more to this that perhaps I'm missing? > > > > > > Making the code more maintainable by the community, as ActiveMQ is an > > > Apache *community* project, is the goal. As for it being maintained > for 7 > > > years, that's great. However, I'm sure you'll agree it's not perfect, > and > > > community improvements are welcome. > > > > > > Art > > > > > > > > > On Wed, Nov 28, 2018 at 3:30 AM Gary Tully <gary.tu...@gmail.com> > wrote: > > > > > > > Jamie, > > > > you are missing my point. it is a tradeoff plain and simple. easier > to > > > > maintain for who? It has been carefully maintained for more than 7 > > > > years. > > > > Do refactoring at the beginning of a release cycle, not at the end. > > > > diffs between 5.15.x and 5.16 will be meaningless otherwise. > > > > push out 5.16.0, which has loads of incremental (non refactored) > small > > > > changes. Then refactor away on master for 5.17.0 and make it better > in > > > > any way that works for the future and for you. > > > > On Tue, 27 Nov 2018 at 15:34, Jamie G. <jamie.goody...@gmail.com> > wrote: > > > > > > > > > > Hi Gary, > > > > > > > > > > To address your concerns: > > > > > > > > > > 1. The code is being cleaned up, not completely rewritten. This is > > > > > making it easier to maintain over the monolithic classes. It's > also > > > > > why it was brought to the community… to review it and be sure the > > > > > changes are just cleaning it up. There was no intent to change the > > > > > logic for the reason that you suggested. > > > > > > > > > > 2. I answered above, its easy on the back port as we can just make > it > > > > > a part of 5.15.9 (too my understanding the community supports > master > > > > > and the last release branch). There are little differences > between 16 > > > > > and 15.9 in the KahaDB realm. So placing it in 15.9 does not > change > > > > > any way it operates or works. It only cleans up the readability of > > > > > the code. > > > > > > > > > > > > > > > "A dream you dream alone is only a dream. A dream you dream > together > > > > > is reality." > > > > > > > > > > ― John Lennon > > > > > > > > > > > > > > > Cheers, > > > > > Jamie > > > > > On Tue, Nov 27, 2018 at 8:29 AM Gary Tully <gary.tu...@gmail.com> > wrote: > > > > > > > > > > > > Hi Jamie G, > > > > > > There are a few trade offs to consider: > > > > > > 1) those familiar with the code will have to reacquaint > themselves > > > > > > with anything that is refactored > > > > > > 2) backporting fixes will be more difficult when the code > structure > > > > changes > > > > > > > > > > > > Of the two, I think #2 is more critical. > > > > > > > > > > > > On #1: > > > > > > context builds up over time and code structure is an integral > part of > > > > > > that, for better or for worse. > > > > > > Switching context is not something us humans like doing, most > > > > > > especially when stability is a key concern. > > > > > > > > > > > > Refactoring with purpose (for a new feature) can be great, doing > it at > > > > > > this stage for readability may be less great. > > > > > > > > > > > > Mr. W. B. Yeats put it nicely: "tread softly because you tread > on my > > > > dreams" > > > > > > > > > > > > s/dreams/mental model/ > > > > > > On Mon, 26 Nov 2018 at 19:44, Christopher Shannon > > > > > > <christopher.l.shan...@gmail.com> wrote: > > > > > > > > > > > > > > I didn't say I definitely wouldn't support it but I just want > to > > > > make sure > > > > > > > we are careful and the benefits of the refactor (in this case > > > > improved > > > > > > > maintainability) are really going to be worth the risk > specifically > > > > because > > > > > > > of the component being touched. Too often things get > committed and > > > > then > > > > > > > issues are uncovered with the patch later that were missed. > > > > > > > > > > > > > > Some parts of the broker are critical and this is one of them > > > > because a bug > > > > > > > that corrupts a store could lead to losing lots of production > data > > > > which is > > > > > > > a very different type of bug than a random web console bug or > > > > transport > > > > > > > error that just causes an error with a single client or with a > single > > > > > > > message. Granted the risk of a critical bug being introduced > with a > > > > > > > refactor like this is not very high but if there is one it > could have > > > > > > > pretty bad consequences. > > > > > > > > > > > > > > Now all that being said ... as long as we are careful to make > sure > > > > all > > > > > > > tests pass and have a few people thoroughly review it (such as > Gary > > > > who has > > > > > > > the most experience out of anyone in KahaDB) then I would +1 > the > > > > change for > > > > > > > a 5.16.0 release. > > > > > > > > > > > > > > On Mon, Nov 26, 2018 at 2:07 PM Arthur Naseef <a...@amlinv.com> > > > > wrote: > > > > > > > > > > > > > > > Improving the existing code is a great goal. > > > > > > > > > > > > > > > > While cleaning up code is nice KahaDB has gotten pretty > stable > > > > over the > > > > > > > > > years and doing a bunch of refactoring just opens it up to > new > > > > bugs that > > > > > > > > > have to be fixed. Fixing bugs is not a problem however I > tend > > > > to be more > > > > > > > > > sensitive to store related changes because of the possible > data > > > > loss or > > > > > > > > > corruption issues to production data that can occur from > store > > > > bugs vs > > > > > > > > some > > > > > > > > > other random bug in the broker. > > > > > > > > > > > > > > > > > > > > > > > > > I understand the desire to avoid the risk of introducing > bugs. > > > > However, as > > > > > > > > long as the project is active and maintained, that really > isn't a > > > > valid > > > > > > > > approach to its maintenance overall. > > > > > > > > > > > > > > > > So that leads us to the question of risk mitigation. > Build-time > > > > tests are > > > > > > > > an industry standard, and ActiveMQ certainly has a large > number of > > > > such > > > > > > > > tests. Code reviews are another best-practice, and there are > > > > multiple > > > > > > > > individuals looking at these code changes now. More are > always > > > > welcome, > > > > > > > > and access is certainly not a problem! > > > > > > > > > > > > > > > > If there are specific concerns within the code changes, let's > > > > discuss > > > > > > > > them. It'll be great to have actual technical discussions! > > > > > > > > > > > > > > > > As for the concern that this is the broker's storage, similar > > > > arguments > > > > > > > > could be made for just about any part of the code. Reliable > > > > handling of > > > > > > > > messages is ActiveMQ's core benefit to its users. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > FWIW, the current community goal is for ActiveMQ Artemis to > > > > become > > > > > > > > ActiveMQ > > > > > > > > > > > > > > > > 6.x when acceptable feature parity is reached (which is > actively > > > > being > > > > > > > > > worked on). > > > > > > > > > > > > > > > > > > > > > > > > > Whether Artemis will eventually become ActiveMQ 6.x is not > > > > pertitent to > > > > > > > > this discussion. Let's not open that discussion as part of > this > > > > technical > > > > > > > > code conversation. > > > > > > > > > > > > > > > > Making the existing code base, which has heavy usage in the > > > > industry, more > > > > > > > > maintainable is always a good goal to achieve. And having > more > > > > folks in > > > > > > > > the community engaging in working on the project can only > benefit > > > > the > > > > > > > > entire community regardless of the long-term destination. > > > > > > > > > > > > > > > > Art > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 26, 2018 at 10:22 AM Justin Bertram < > > > > jbert...@apache.org> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > FWIW, the current community goal is for ActiveMQ Artemis to > > > > become > > > > > > > > ActiveMQ > > > > > > > > > 6.x when acceptable feature parity is reached (which is > actively > > > > being > > > > > > > > > worked on). > > > > > > > > > > > > > > > > > > > > > > > > > > > Justin > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Nov 26, 2018 at 11:01 AM Jamie G. < > > > > jamie.goody...@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > The idea here is to ensure that it’s development and > > > > maintenance > > > > > > > > > > moving forward is easier as we work to make it better in > the > > > > future. > > > > > > > > > > > > > > > > > > > > KahaDB currently has massive classes (KahaDBStore, > > > > MessageDatabase) > > > > > > > > > > and code base and is extremely hard to follow. My > desire to > > > > do this > > > > > > > > > > was to make this so we could make the continued > maintenance > > > > easier and > > > > > > > > > > also make it more inviting to improvements. The unit > tests > > > > all pass, > > > > > > > > > > so as long as we have a good solid testing coverage, the > risks > > > > are not > > > > > > > > > > huge. If a bug appears to be introduced, than we may > have > > > > uncovered a > > > > > > > > > > testing hole - finding these is a good thing. > > > > > > > > > > > > > > > > > > > > Since we are going to continue to support ActiveMQ moving > > > > forward, > > > > > > > > > > it’s a good idea to make it more maintainable. My > motivation > > > > to doing > > > > > > > > > > this was spurred by the recent JIRAs surrounding KahaDB I > > > > helped out > > > > > > > > > > upon. I really believe this is a good improvement to > help > > > > moving > > > > > > > > > > ActiveMQ forward. > > > > > > > > > > On Mon, Nov 26, 2018 at 12:33 PM Christopher Shannon > > > > > > > > > > <christopher.l.shan...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > I'm not really sure this is worthwhile or something we > want > > > > to do...I > > > > > > > > > > would > > > > > > > > > > > have to think about it more before I gave it a +1. > > > > > > > > > > > > > > > > > > > > > > While cleaning up code is nice KahaDB has gotten pretty > > > > stable over > > > > > > > > the > > > > > > > > > > > years and doing a bunch of refactoring just opens it > up to > > > > new bugs > > > > > > > > > that > > > > > > > > > > > have to be fixed. Fixing bugs is not a problem > however I > > > > tend to be > > > > > > > > > more > > > > > > > > > > > sensitive to store related changes because of the > possible > > > > data loss > > > > > > > > or > > > > > > > > > > > corruption issues to production data that can occur > from > > > > store bugs > > > > > > > > vs > > > > > > > > > > some > > > > > > > > > > > other random bug in the broker. > > > > > > > > > > > > > > > > > > > > > > On Sun, Nov 25, 2018 at 11:59 PM Jean-Baptiste Onofré < > > > > > > > > j...@nanthrax.net > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > OK, got it. It's more a syntax/codebase organization > > > > refactoring. > > > > > > > > > > > > > > > > > > > > > > > > If there's no impact on the behavior and features, > +1 from > > > > my side. > > > > > > > > > > > > > > > > > > > > > > > > Regards > > > > > > > > > > > > JB > > > > > > > > > > > > > > > > > > > > > > > > On 25/11/2018 21:21, Jamie G. wrote: > > > > > > > > > > > > > Initially its to make KahaDB classes easier to > read & > > > > maintain. > > > > > > > > > > > > > Eventually it will help in features/performance; > smaller > > > > classes > > > > > > > > > are > > > > > > > > > > > > > easier to grok, easier to see improvements. > > > > > > > > > > > > > > > > > > > > > > > > > > Instead of trying to refactor all of it in one go, > I'm > > > > taking the > > > > > > > > > > > > > approach of one area at a time. > > > > > > > > > > > > > > > > > > > > > > > > > > One pass for breaking out objects. > > > > > > > > > > > > > Another pass for small functional improvements. > > > > > > > > > > > > > Perhaps future passes for new Java features (bring > all > > > > code up to > > > > > > > > > > Java > > > > > > > > > > > > > 8 perhaps?). > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, Nov 25, 2018 at 4:32 PM Jean-Baptiste > Onofré < > > > > > > > > > > j...@nanthrax.net> > > > > > > > > > > > > wrote: > > > > > > > > > > > > >> > > > > > > > > > > > > >> Hi Jamie, > > > > > > > > > > > > >> > > > > > > > > > > > > >> That's interesting. > > > > > > > > > > > > >> > > > > > > > > > > > > >> What's the rationale behind the refactoring ? New > > > > features or > > > > > > > > perf > > > > > > > > > > > > >> improvements ? > > > > > > > > > > > > >> > > > > > > > > > > > > >> Regards > > > > > > > > > > > > >> JB > > > > > > > > > > > > >> > > > > > > > > > > > > >> On 25/11/2018 20:16, Jamie G. wrote: > > > > > > > > > > > > >>> Hi All, > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> I've taken some time to prototype a refactored > > > > KahaDBStore > > > > > > > > class: > > > > > > > > > > > > >>> > > > > https://github.com/jgoodyear/activemq/tree/KahaDBRefactor > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> As KahaDBStore exists in Master, it contains 7 > internal > > > > > > > > classes, > > > > > > > > > > over > > > > > > > > > > > > >>> some 1677 lines of code. > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> In my refactor branch I've separated out those > classes > > > > into > > > > > > > > their > > > > > > > > > > own > > > > > > > > > > > > >>> files, and applied some gentle clean code > practices to > > > > help > > > > > > > > make > > > > > > > > > > these > > > > > > > > > > > > >>> files easier to read and maintain. > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> I'd like to gather feed back from the community; > I've > > > > taken > > > > > > > > care > > > > > > > > > to > > > > > > > > > > > > >>> change functionality as little as possible - the > aim > > > > here is to > > > > > > > > > > reduce > > > > > > > > > > > > >>> complexity and improve maintainability. If the > > > > community feels > > > > > > > > > > this is > > > > > > > > > > > > >>> a worth while goal than I'll open a card on Jira > & > > > > prepare a > > > > > > > > PR. > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> Notes: > > > > > > > > > > > > >>> ActiveMQ KahaDB Store and ActiveMQ-Unit-Tests > suites > > > > remain > > > > > > > > > > passing > > > > > > > > > > > > >>> after refactor. > > > > > > > > > > > > >>> > > > > > > > > > > > > >>> Cheers, > > > > > > > > > > > > >>> Jamie > > > > > > > > > > > > >>> > > > > > > > > > > > > >> > > > > > > > > > > > > >> -- > > > > > > > > > > > > >> Jean-Baptiste Onofré > > > > > > > > > > > > >> jbono...@apache.org > > > > > > > > > > > > >> http://blog.nanthrax.net > > > > > > > > > > > > >> Talend - http://www.talend.com > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > Jean-Baptiste Onofré > > > > > > > > > > > > jbono...@apache.org > > > > > > > > > > > > http://blog.nanthrax.net > > > > > > > > > > > > Talend - http://www.talend.com > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Clebert Suconic
