Il dom 29 dic 2019, 16:02 Benjamin Marwell <bmarw...@gmail.com> ha scritto:
> PR is updated. > > I found out that they might switch to semantic versioning as soon as they > have cleaned the API: https://github.com/checkstyle/checkstyle/issues/3709 > > That also means: the faster the current PR is integrated, the better - less > breaking changes for users. > And we should cut a release of Maven plugin Enrico > > > On Mon, 23 Dec 2019, 18:00 Romain Manni-Bucau, <rmannibu...@gmail.com> > wrote: > > > Le lun. 23 déc. 2019 à 17:51, Benjamin Marwell <bmarw...@gmail.com> a > > écrit : > > > > > 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. > > > > > > > We can also just try catch the missing method, will not break us and work > > with both version > > > > > > > 3 is implemented via 2. 😉 > > > > > > > It is more checkstyle side vs maven side but can be > > > > > > > > > > > > > > > On Mon, 23 Dec 2019, 16:50 Romain Manni-Bucau, <rmannibu...@gmail.com> > > > 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 <bmarw...@gmail.com> > 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, <f.mod...@gmx.net> 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 < > > > > > > rmannibu...@gmail.com>: > > > > > > >> > > > > > > >> 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 < > > > bmarw...@gmail.com> > > > > 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, < > > > > > rmannibu...@gmail.com> > > > > > > >>> 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 < > > > > bmarw...@gmail.com> > > > > > 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, < > > > eolive...@gmail.com > > > > > > > > > > > >>> 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 < > > > bmarw...@gmail.com> > > > > > 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: dev-unsubscr...@maven.apache.org > > > > > > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > > > > > For additional commands, e-mail: dev-h...@maven.apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > >
