Thanks everyone for their feedback! I have created this Jira ticket to track https://issues.apache.org/jira/projects/AMQ/issues/AMQ-9648?filter=allissues
Thanks, Ken On Mon, Jan 20, 2025 at 4:15 AM Christopher Shannon < christopher.l.shan...@gmail.com> wrote: > I already responded once with a short response saying I don't think it's a > good idea to remove the parser but I will elaborate more. Reading over this > thread again I think there are really two independent things being > discussed here and should be considered separately. > > 1) Removing javacc as a build time dependency > > I'm a -1 on this because I still do not understand the motivation. I 100% > agree with Justin's response. It's just a build time dependency and we've > had it forever and it's causing zero problems. What if we need to change > the generated parser? Just because we haven't for a long time doesn't mean > we won't need to. Removing it means you'd have to manually change the > output or switch to something else. If the dependency is old then we handle > it like every other dependency, we upgrade. > > I see mention of a maintenance burden but I don't see how there really is > one. Keeping the dependency up to date shouldn't be any different than > updating any of the other tons of dependencies ActiveMQ has. I'd argue it's > a lot less of a burden considering we've not updated in a very long time > and it's not shipped with the broker. We should just update to the latest > version just like Artemis did. > > 2) Commiting the generated source code to the repo > > I'm -0 on this as I don't think it is necessary because the generated > source is fast and only a few files but I guess it could be discussed more > if there was some benefit. In this case we would just set up the parser to > generate only if there's a change (so maybe move it into an optional > profile). Many projects do this where you only run the generator when > there's a change and then commit the output to avoid having to run it each > time. This is pretty common when generating serializers for frameworks like > protobuf or Thrift. This is also how we handle OpenWire, we have (well had, > the current openwire generator is broken with new JDKs which is why we are > working on the new one) the ability to generate a new version when we need > to with a maven profile and just commit the generated source. > > > > On Sat, Jan 18, 2025 at 10:39 AM Jean-Baptiste Onofré <j...@nanthrax.net> > wrote: > > > Hi > > > > You are right Art: there’s no issue for now. > > We are not in the rush to update. I would just create a Jira to track it > > for later purpose. > > > > Regards > > JB > > > > > > Le sam. 18 janv. 2025 à 14:24, Arthur Naseef <artnas...@apache.org> a > > écrit : > > > > > Is the generated code problematic, or are we talking hypotheticals > here? > > > > > > Not trying to discourage updating - but instead trying to understand > need > > > here: > > > why does javacc need to be updated? > > > > > > Ther javacc grammar file is the original code, and the generated java > is > > an > > > output of that. If we want to hand-maintain a parser, we probably want > > > something more readable/maintainable than javacc output. > > > > > > Art > > > > > > On Sat, Jan 18, 2025, 12:38 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > > wrote: > > > > > > > Do we really need to generate this code ? > > > > > > > > It's a dependency of the build (not the packages dependency), but it > > > > could be problematic regarding the code generated for updated JDK > > > > versions (JDK11 and beyond, especially JDK23 at some point). > > > > > > > > As we don't generate the code often, I don't see the benefit compared > > > > to just having the source. I don't think it's an urgent matter, but > > > > it's worth keeping this in mind and I think it would be a good idea > to > > > > remove this build dep. > > > > > > > > Regards > > > > JB > > > > > > > > On Sat, Jan 18, 2025 at 12:15 AM Christopher Shannon > > > > <christopher.l.shan...@gmail.com> wrote: > > > > > > > > > > I don't think there is a good reason to get rid of dependency > either, > > > we > > > > > absolutely need to be able to build a new version going forward. > Just > > > > > because it hasn't changed doesn't mean it won't. > > > > > > > > > > On Fri, Jan 17, 2025 at 3:20 PM Arthur Naseef < > artnas...@apache.org> > > > > wrote: > > > > > > > > > > > I agree with Justin here. What is the need to remove the > > dependency > > > on > > > > > > javacc - especially since it is build-time only? > > > > > > > > > > > > Art > > > > > > > > > > > > > > > > > > On Fri, Jan 17, 2025 at 1:11 PM Justin Bertram < > > jbert...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > In what sense is JavaCC a "dependency of the activemq-client > > > > package"? > > > > > > It's > > > > > > > not a Maven dependency, and it's not shipped with the broker. > > It's > > > > simply > > > > > > > part of the build process and represents a near-zero > maintenance > > > > burden. > > > > > > > > > > > > > > I'm against checking in the generated source and removing the > > > > integration > > > > > > > with JavaCC for the following reasons: > > > > > > > > > > > > > > - You never know what changes will be required in the future. > > > > Generally > > > > > > > speaking, you'd want to modify the JavaCC input rather than the > > > > JavaCC > > > > > > > output in that case. > > > > > > > - If there is ever any improvement to JavaCC we won't benefit > > > from > > > > it. > > > > > > > - There is no real downside to keeping the existing structure > > in > > > > place. > > > > > > > > > > > > > > Artemis uses the same basic process to generate the selector > > > parser, > > > > and > > > > > > it > > > > > > > uses JavaCC 7.0.13 without issue. > > > > > > > > > > > > > > What is the benefit of removing the integration and checking in > > the > > > > > > > generated code? > > > > > > > > > > > > > > > > > > > > > Justin > > > > > > > > > > > > > > On Fri, Jan 17, 2025 at 3:49 AM Ken Liao <kenlia...@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > Hi folks, > > > > > > > > > > > > > > > > Recently, I am diving into the SelectorParser.java generated > by > > > > > > javacc. I > > > > > > > > am wondering, do we want to keep maintaining javacc as a > > > > dependency of > > > > > > > the > > > > > > > > activemq-client package? > > > > > > > > > > > > > > > > In another word, the grammar of the JMS selector hasn't > changed > > > > (last > > > > > > > time > > > > > > > > the change made to the grammar definition file > > SelectorParser.jj > > > is > > > > > > > > changing the namespace to jakarta in the main branch). Would > it > > > be > > > > > > easier > > > > > > > > to just commit the generated java file as source and remove > the > > > > javacc > > > > > > > > dependency? > > > > > > > > > > > > > > > > If we do want to keep it as a dependency, the latest stable > > > > release of > > > > > > > > javacc is version 7. I can upgrade javacc to version 7 to > check > > > if > > > > it > > > > > > > > breaks the build and tests. I will create a PR on it soon if > > > > there's no > > > > > > > > objection. > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Ken > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: dev-unsubscr...@activemq.apache.org > > > > For additional commands, e-mail: dev-h...@activemq.apache.org > > > > For further information, visit: https://activemq.apache.org/contact > > > > > > > > > > > > > > > > > >
