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
> > > >
> > > >
> > >
> >
>
