Re: More checkstyle API changes
Can't wait to see it integrated, as I will add another PR based on the
previous one to emit a message and deprecate that new method.
Do I need to do something to have the PR merged?
On Sun, 29 Dec 2019, 17:38 Enrico Olivelli, wrote:
> Il dom 29 dic 2019, 16:02 Benjamin Marwell 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,
> > wrote:
> >
> > > Le lun. 23 déc. 2019 à 17:51, Benjamin Marwell 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 >
> > 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,
> 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
Re: More checkstyle API changes
Il dom 29 dic 2019, 16:02 Benjamin Marwell 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,
> wrote:
>
> > Le lun. 23 déc. 2019 à 17:51, Benjamin Marwell 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,
> > > 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
> 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, 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
> > > > > > >>>
Re: More checkstyle API changes
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.
On Mon, 23 Dec 2019, 18:00 Romain Manni-Bucau,
wrote:
> Le lun. 23 déc. 2019 à 17:51, Benjamin Marwell 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,
> > 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 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, 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).
>
Re: More checkstyle API changes
On Mon 23 Dec 2019 at 15:44, Benjamin Marwell wrote:
> 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.
Let’s not bump any plugin to 4.x at this time. Better if we can hold that
version number for plugins using Maven 4 as a baseline
>
>
> 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, 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 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
> 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,
> > >>> 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
Re: More checkstyle API changes
Le lun. 23 déc. 2019 à 17:51, Benjamin Marwell 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,
> 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 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, 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
Re: More checkstyle API changes
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,
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 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, 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
> 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
Re: More checkstyle API changes
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 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, 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 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
> 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,
> > >>> wrote:
> > >> Ben,
> > >> What about having a release of checkstyle with all of the
> requested
> > > changes
Re: More checkstyle API changes
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, 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 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,
> >>> 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 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,
> >>> 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 ha
> >> scritto:
> >>
> >>> Hi all,
> >>>
> >>> The checkstyle team is waiting for my PR:
> >>>
> >>>
Re: More checkstyle API changes
+1 to move the plugin ... otherwise i dont see us breaking it since we document - and cant help users doing it anyway - how to chnage the checkstyle version, so we must support upgrades/downgrades as transparently as possible, we are not in the mode "1 plugin version/ 1 checkstyle version" so not sure we have the choice if they refuse the plugin. Le lun. 23 déc. 2019 à 15:35, Falko Modler a écrit : > 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 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, > >>> 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 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, > >>> 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 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
Re: More checkstyle API changes
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 : 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 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, 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 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, 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 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
Re: More checkstyle API changes
Robert>last time their argument was that they didn't have enough committers to also maintain a maven plugin TL;DR: I'm neither Maven nor Checkstyle committer, and I'm inclined that the PR should be declined. The case is as follows: there's a method in Checkstyle which is a no-op for 3 years or so. For some reasons (aesthetics?), Checkstyle developers decided to drop the method. They know the method is used by third parties. ^^^ What is the reason to drop a method you know is used by third parties? The consequences would be: 1) Maven committers and PMC would have to release a new plugin version (stage, verify, vote, publish dist) 2) Lots of users will face an error while trying to use never Checkstyle version. For example, In 99% of the cases, I just upgrade Checkstyle version, and I have no clue what is my version of maven-checkstyle-plugin. 3) Of course, their builds will fail. Of course, they will be able to Google the error and they will learn they need to bump maven-checkstyle-plugin. I'm not sure Checkstyle developers understand ASF release process, and I'm not sure they fully understand that "making a release" is not like a "pushing Git tag" :( I'm not sure Checkstyle developers understand they are creating a painful experience for the end-users :( This causes pain for Maven committers, PMCs, and for end-users who will blame all the issues on maven-plugin (because it was "too old, not releasing soon enough, etc, etc"). It does impact Maven in a negative way (not just the plugin), because, you know, at the end of the day it will be like "-- Maven never works -- Realy? What's that? -- Don't remember, there was a problem with one of the Maven core plugins..." What I mean here is like no-one would file a single issue to Checkstyle team for "breaking backward compatibility". I guess it would be better if they will be responsible for the decisions they make. It looks like there are two ways to proceed here: A) Decline the PR, and keep the method at Checkstyle side. Then no changes to the plugin are needed, and everything is good. B) Ensure the plugin version is exactly the same as the main Checsktyle version. Then end-users could specify a single property like "checkstyle.version", and use it for both plugin and checkstyle (well, they won't need to specify checkstyle then), and it would ensure the plugin is always compatible with Checkstyle engine. C) Accept the PR, and make sure Checkstyle will keep the method available for 5 more years (or so) to ensure everybody has migrated to the new plugin version. I guess "B" implies the plugin must be moved under Checkstyle repository so they could perform simultaneous releases (engine + Maven plugin) I do not think "C" is good because, well, I doubt Checkstyle developers are ready to wait 5 more years, and it looks like they are going to drop the method as soon as the new plugin version is released. Robert>To me moving the plugin would help both projects In practice, Checkstyle developers might be not that familiar with "Maven conventions": ways to resolve ClassPath, ways to configure properties, etc. So it makes a certain sense to review the plugin implementation by someone who understands how Maven should feel. However, "ASF release" overheads seem to be very visible (to PMC), especially when the only things to review are like "oh, an empty method is dropped" :( Vladimir
Re: More checkstyle API changes
To me moving the plugin would help both projects. IIRC the last time their argument was that they didn't have enough committers to also maintain a maven plugin. That's still a little bit weird if you compare that ratio (active committers per LOCs) with the Maven Project. Robert On 23-12-2019 14:30:09, Benjamin Marwell wrote: Even if they don't accept it... It would be helpful if they used semantic versioning. That way maintaining that plugin would be much less of a hazzle. As I created the PR, I'm going to ask them if they would develop the plugin on their own, if you are okay with this. On Mon, 23 Dec 2019, 13:57 Robert Scholte, wrote: > I'd like to see it the other way around: move the plugin to the checkstyle > team (with or without help from someone of the maven team). > > We've seen this more often in the past where products took over the > plugins from Apache Maven or Codehaus Mojo and it worked out well. > Now they can make the plugin part of the release and keep them in sync. > > Robert > > On 23-12-2019 13:21:34, Enrico Olivelli wrote: > Il lun 23 dic 2019, 12:58 Mark Struberg ha > scritto: > > > 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. > > > > This is exactly what I meant. > I think is it fine to release maven checkstyle plugin with a new version of > checkstyle. > Maybe in the future any of checkstyle team will become a Maven committer > and thisI will ease a lot this work > > > Enrico > > > > 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 > > >: > > > > > > 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 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, > > >> 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 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, > > >> 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 ha > > > scritto: > > > > > >> Hi all, > > >> > > >>
Re: More checkstyle API changes
Even if they don't accept it... It would be helpful if they used semantic versioning. That way maintaining that plugin would be much less of a hazzle. As I created the PR, I'm going to ask them if they would develop the plugin on their own, if you are okay with this. On Mon, 23 Dec 2019, 13:57 Robert Scholte, wrote: > I'd like to see it the other way around: move the plugin to the checkstyle > team (with or without help from someone of the maven team). > > We've seen this more often in the past where products took over the > plugins from Apache Maven or Codehaus Mojo and it worked out well. > Now they can make the plugin part of the release and keep them in sync. > > Robert > > On 23-12-2019 13:21:34, Enrico Olivelli wrote: > Il lun 23 dic 2019, 12:58 Mark Struberg ha > scritto: > > > 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. > > > > This is exactly what I meant. > I think is it fine to release maven checkstyle plugin with a new version of > checkstyle. > Maybe in the future any of checkstyle team will become a Maven committer > and thisI will ease a lot this work > > > Enrico > > > > 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 > > >: > > > > > > 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 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, > > >> 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 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, > > >> 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 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.
Re: More checkstyle API changes
I'd like to see it the other way around: move the plugin to the checkstyle team (with or without help from someone of the maven team). We've seen this more often in the past where products took over the plugins from Apache Maven or Codehaus Mojo and it worked out well. Now they can make the plugin part of the release and keep them in sync. Robert On 23-12-2019 13:21:34, Enrico Olivelli wrote: Il lun 23 dic 2019, 12:58 Mark Struberg ha scritto: > 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. > This is exactly what I meant. I think is it fine to release maven checkstyle plugin with a new version of checkstyle. Maybe in the future any of checkstyle team will become a Maven committer and thisI will ease a lot this work Enrico > 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 > >: > > > > 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 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, > >> 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 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, > >> 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 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? > >> > >>
Re: More checkstyle API changes
Il lun 23 dic 2019, 12:58 Mark Struberg ha scritto: > 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. > This is exactly what I meant. I think is it fine to release maven checkstyle plugin with a new version of checkstyle. Maybe in the future any of checkstyle team will become a Maven committer and thisI will ease a lot this work Enrico > 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 >: > > > > 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 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, > >> 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 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, > >> 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 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 > >
Re: More checkstyle API changes
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 : > > 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 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, >> 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 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, >> 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 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
Re: More checkstyle API changes
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 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, > 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 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, > 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 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 > > > > > > > > > > > > > > >
Re: More checkstyle API changes
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, 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 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, 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 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 > > > > > > > > > >
Re: More checkstyle API changes
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 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, 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 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 > > > > > >
Re: More checkstyle API changes
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, 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 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 > > >
Re: More checkstyle API changes
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 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 >
More checkstyle API changes
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
