Sounds good to me. A new major is not needed for my PR, but needed for future versions of checkstyle. Depends on how fast they actually remove that method.
3 is implemented via 2. 😉 On Mon, 23 Dec 2019, 16:50 Romain Manni-Bucau, <[email protected]> wrote: > What about steps? > > 1. Ask them to grab the plugin (with our support pby) > 2. If 1 fails, semver and we align on that somehow in our versioning > (likely a new major?) > 3. More tolerant fallback respecting user configured version, no user > breaking/enforced change (it hurts way too much even if nicer for us) > > Wdyt? > > > Le lun. 23 déc. 2019 à 16:44, Benjamin Marwell <[email protected]> a > écrit : > > > Furthermore, > > > > if we do not drop using that method, maven will throw an exception. Maven > > will, not checkstyle. > > > > I do not think that should be happening (from a user perspective). > > > > It's an easy fix for maven. As soon as the checkstyle Plugin required > > checkstyle 8.24 or higher, it the plugin should go to 4.x (new major > > version). Simple as that. > > > > I am even willing to implement a Checkstyle version check to explain the > > incompatibilities of checkstyle 8.24 and above. It's not much work and > will > > be helpful to users. Seeing some error ("TreeeWalker does not allow the > > subelement LineLength") is not helpful by itself. It's easy to document > and > > easy to detect. > > > > Also, why not ask the checkstyle team to adapt semantic versioning? > Asking > > doesn't cost anything afaik. > > > > > > On Mon, 23 Dec 2019, 15:35 Falko Modler, <[email protected]> wrote: > > > > > Hi Mark, > > > > > > > The maven-checkstyle-plugin is rather pretty much hardcoded to a > > > specific checkstyle version. While you _could_ technically exchange the > > > checkstyle dependency it is not really intended. > > > > > > > > > Are you sure it is not really intended? It is actually _documented_: > > > > > > > > > https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/upgrading-checkstyle.html > > > > > > I've been using it this way for many years and I am sure many other > > > users have done the same. > > > > > > Best regards, > > > > > > Falko > > > > > > > > > Am 23.12.2019 um 12:57 schrieb Mark Struberg: > > > > But the main purpose is not to have multiple frameworks run with it. > > > That's the main difference to surefire. > > > > > > > > The maven-checkstyle-plugin is rather pretty much hardcoded to a > > > specific checkstyle version. While you _could_ technically exchange the > > > checkstyle dependency it is not really intended. > > > > > > > > The 'compatibility' layer is rather only important for the checkstyle > > > configuration. That part should really remain stable. If not, we have > to > > up > > > to major version. > > > > > > > > LieGrue, > > > > strub > > > > > > > > > > > >> Am 23.12.2019 um 12:34 schrieb Romain Manni-Bucau < > > > [email protected]>: > > > >> > > > >> Point is it is the only way to not break end user since it gives us > > the > > > >> control of which version to select depending user config and java > > > version. > > > >> Which we dont ask any change to user im fine either ways though. > > > >> > > > >> Le lun. 23 déc. 2019 à 12:28, Benjamin Marwell <[email protected]> > a > > > >> écrit : > > > >> > > > >>> I not think that would be a benefit, because removing the class > > loader > > > call > > > >>> will also work with older versions of checkstyle. > > > >>> Also, of the plugin is just a wrapper, why even bother to detect if > > the > > > >>> checkstyle.xml and checkstyle dependency will work together? > > > >>> > > > >>> Or am I missing something? > > > >>> > > > >>> On Mon, 23 Dec 2019, 09:32 Romain Manni-Bucau, < > > [email protected]> > > > >>> wrote: > > > >>> > > > >>>> What about loading checkstyle from a dependency resolver and use a > > > custom > > > >>>> classloader with an integration per version (a bit like surefire). > > It > > > >>>> enables to use any version and even detect an user override in > > plugin > > > >>> deps. > > > >>>> Le lun. 23 déc. 2019 à 09:27, Benjamin Marwell < > [email protected]> > > a > > > >>>> écrit : > > > >>>> > > > >>>>> Hi Enrico, > > > >>>>> > > > >>>>> that would mean a lot of incompatibilities which I wanted to > avoid. > > > >>>>> If the checkstyle jar is updated first (8.xx), maven users > wouldn't > > > be > > > >>>> able > > > >>>>> to use a current version for quite a while, because the Maven > > plugin > > > >>>> would > > > >>>>> throw NoSuchMethodExceptions. I wanted to avoid this. > > > >>>>> > > > >>>>> On the other hand, if the Maven plugin gets updated and released > > > first, > > > >>>>> there is more time for users to migrate to a more recent maven > > > plugin. > > > >>>>> Hence my PR. > > > >>>>> > > > >>>>> I really do not see the benefit of updating the checkstyle jar > > first > > > >>> and > > > >>>>> this having a period of time where Maven users cannot use a > recent > > > >>>> version > > > >>>>> of checkstyle at all. > > > >>>>> > > > >>>>> Maybe I did express the issue with checkstyle 8.24 in a wrong > way. > > > >>> Users > > > >>>>> can already use it if they rewrite their checkstyle.xml. it's > just > > > that > > > >>>> the > > > >>>>> maven plugin should not update the default checkstyle version to > > not > > > >>>> break > > > >>>>> any default setups and force users to rewrite their checks. > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> On Mon, 23 Dec 2019, 08:45 Enrico Olivelli, <[email protected] > > > > > >>> wrote: > > > >>>>>> Ben, > > > >>>>>> What about having a release of checkstyle with all of the > > requested > > > >>>>> changes > > > >>>>>> and then update maven plugin and then release it? > > > >>>>>> Checkstyle maven plugin is just a wrapper over checkstyle > library. > > > >>>>>> > > > >>>>>> The best way would be that you (or anyone from Checkstyle) > create > > a > > > >>> PR > > > >>>>> when > > > >>>>>> you are ready with the new release. > > > >>>>>> > > > >>>>>> I will be happy to help you move forward with this change and > cut > > a > > > >>>>> release > > > >>>>>> Cheers > > > >>>>>> Enrico > > > >>>>>> > > > >>>>>> Il lun 23 dic 2019, 07:21 Benjamin Marwell <[email protected]> > > ha > > > >>>>>> scritto: > > > >>>>>> > > > >>>>>>> Hi all, > > > >>>>>>> > > > >>>>>>> The checkstyle team is waiting for my PR: > > > >>>>>>> > > > >>>>>>> https://github.com/apache/maven-checkstyle-plugin/pull/18 > > > >>>>>>> > > > >>>>>>> The problem is, that they want to remove a method. If they do > > this > > > >>>> too > > > >>>>>>> early, maven users will not be able to update the checkstyle > > > >>> version > > > >>>>>>> anymore. > > > >>>>>>> > > > >>>>>>> Also, the maven Checkstyle plugin cannot ship a Checkstyle > > version > > > >>>>> beyond > > > >>>>>>> 8.23 because of breaking changes. There is also an issue for > > this. > > > >>>>>>> > > > >>>>>>> This really needs some attention by someone with more > > > >>> responsibility. > > > >>>>>>> Please keep in mind that there is already a jira issue about > the > > > >>> 8.24 > > > >>>>>>> incompability. I commented that they should have made it a > major > > > >>>>> version, > > > >>>>>>> and maybe the checkstyle plugin will have to jump to a new > major > > > >>>>> release > > > >>>>>> at > > > >>>>>>> some point? > > > >>>>>>> > > > >>>>>>> Thanks for looking into this. > > > >>>>>>> > > > >>>>>>> Ben > > > >>>>>>> > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: [email protected] > > > > For additional commands, e-mail: [email protected] > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [email protected] > > > For additional commands, e-mail: [email protected] > > > > > > > > >
