I see that Romain has selected the "approved" option in GitHub for this PR. Before moving forward, are there any other folks that want to add their opinion, or ask questions?
Jonathan Fisher - you raised some great points, I'd love to hear back on whether you feel they have been addressed or not. Thanks Jon On Wed, Jun 28, 2017 at 10:15 PM, Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > PR(s) updated! Thank you Romain! If anyone else have a chance to look, that > would be awesome. Again, let me know if anything will make the review > easier. > > On Wed, Jun 28, 2017 at 5:09 PM, Jonathan Gallimore < > jonathan.gallim...@gmail.com> wrote: > > > Have to dash out, but will apply these as soon as I'm back. Thanks for > the > > feedback! > > > > Jon > > > > On Wed, Jun 28, 2017 at 5:07 PM, Jonathan Gallimore < > > jonathan.gallim...@gmail.com> wrote: > > > >> I just saw you comments - thanks! Much appreciated. And yes, looks like > >> they came through as comments on the JIRA. That said, I appreciate the > >> follow up on this thread, as I'd like to keep the discussion here (I'm > >> happy to pick up comments on the PR and bring them back here for full > >> visibility). > >> > >> Jon > >> > >> On Wed, Jun 28, 2017 at 5:05 PM, Romain Manni-Bucau < > >> rmannibu...@gmail.com> wrote: > >> > >>> @Jon: commented 2 small things on the PR. More than the fixes itself > >>> which > >>> shouldnt be too hard I'm interested to learn the flow we have when > >>> commenting on github. Know jira should have a comment so a mail too but > >>> on > >>> the "submitter" point of view how does it look like? > >>> > >>> > >>> Romain Manni-Bucau > >>> @rmannibucau <https://twitter.com/rmannibucau> | Blog > >>> <https://blog-rmannibucau.rhcloud.com> | Old Blog > >>> <http://rmannibucau.wordpress.com> | Github < > >>> https://github.com/rmannibucau> | > >>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory > >>> <https://javaeefactory-rmannibucau.rhcloud.com> > >>> > >>> 2017-06-28 18:02 GMT+02:00 Jonathan Gallimore < > >>> jonathan.gallim...@gmail.com> > >>> : > >>> > >>> > Further to this, I have created 2 PRs for further review: > >>> > > >>> > https://github.com/apache/tomee/pull/78 - 1.7.x > >>> > https://github.com/apache/tomee/pull/79 - master > >>> > > >>> > This hopefully addresses the classloader feedback and also the > logging > >>> > feedback. > >>> > > >>> > Any other feedback is gratefully received, and encouraged, from > >>> committers, > >>> > users and anyone else who might be interested. > >>> > > >>> > More importantly, if there is something that I can provide that makes > >>> > review easier, please do let me know. I'm also happy to explain > things > >>> > further if anyone is lost. > >>> > > >>> > Thanks! > >>> > > >>> > Jon > >>> > > >>> > > >>> > On Wed, Jun 28, 2017 at 1:58 PM, Jonathan Gallimore < > >>> > jonathan.gallim...@gmail.com> wrote: > >>> > > >>> > > A big thank you for your comments Romain and Jonathan. Just working > >>> on > >>> > > incorporating those now. I'll push a PR, so its a little easier to > >>> > review. > >>> > > > >>> > > Jonathan - I note your comments about the control flow - is there > >>> > > something I can do help make the review of this easier? I could do > a > >>> > little > >>> > > screencast, or some documentation write up, (or both!) if that > would > >>> be > >>> > > helpful? > >>> > > > >>> > > Thanks! > >>> > > > >>> > > Jon > >>> > > > >>> > > On Mon, Jun 26, 2017 at 8:48 PM, exabrial12 <exabr...@gmail.com> > >>> wrote: > >>> > > > >>> > >> I appreciate the logging lines in the patch that allow the user > some > >>> > >> feedback > >>> > >> as to what happens with the controls. I was trying to think of a > few > >>> > more > >>> > >> that might be useful, but the control flow is a bit complicated to > >>> > >> understand. > >>> > >> > >>> > >> > >>> > >> > >>> > >> -- > >>> > >> View this message in context: http://tomee-openejb.979440.n4 > >>> > >> .nabble.com/MDB-JMX-Control-tp4681962p4681966.html > >>> > >> Sent from the TomEE Dev mailing list archive at Nabble.com. > >>> > >> > >>> > > > >>> > > > >>> > > >>> > >> > >> > > > -- Jonathan Gallimore http://twitter.com/jongallimore http://www.tomitribe.com
