You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Stephan Ewen <se...@apache.org> on 2021/07/13 16:15:06 UTC

[DISCUSS] Address deprecation warnings when upgrading dependencies

Hi all!

I would like to propose that we make it a project standard that when
upgrading a dependency, deprecation issues arising from that need to be
fixed in the same step. If the new dependency version deprecates a method
in favor of another method, all usages in the code need to be replaced
together with the upgrade.

We are accumulating deprecated API uses over time, and it floods logs and
IDEs with deprecation warnings. I find this is a problem, because the
irrelevant warnings more and more drown out the actually relevant warnings.
And arguably, the deprecation warning isn't fully irrelevant, it can cause
problems in the future when the method is actually removed.
We need the general principle that a change leaves the codebase in at least
as good shape as before, otherwise things accumulate over time and the
overall quality goes down.

The concrete example that motivated this for me is the JUnit dependency
upgrade. Pretty much every test I looked at recently is quite yellow (due
to junit Matchers.assertThat being deprecated in the new JUnit version).
This is easily fixed (even a string replace and spotless:apply goes a long
way), so I would suggest we try and do these things in one step in the
future.

Curious what other committers think about this suggestion.

Best,
Stephan

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Stephan Ewen <se...@apache.org>.
@Chesnay - Good question. I think we can be pragmatic there. If you upgrade
Jackson, pick a class that uses it and look for the common methods. If
everything is fine there, it is probably fine overall. If one or two
deprecated method usages are overlooked, no problem, that's not an issue.
If a common method gets deprecated, then chance is high you spot it quickly
when looking at some Jackson usages.

@Yangze - Agreed, we should do the same internally and make sure we keep
our own API use and examples up to date.


On Wed, Jul 14, 2021 at 11:00 AM Chesnay Schepler <ch...@apache.org>
wrote:

> How do you propose to do this in practice?
> Let's say I bump jackson, how would I find all new usages of deprecated
> APIs?
> Build it locally and grep the maven output for jackson?
>
> On 14/07/2021 10:51, Yangze Guo wrote:
> > +1 for fixing deprecation warnings when bumping/changing dependencies.
> >
> > Not only for the dependencies, we also use the deprecated API of Flink
> > itself in `flink-examples` and the document, e.g. the #writeAsText. I
> > think it would be good to do a clean-up for usability. WDYT?
> >
> > Best,
> > Yangze Guo
> >
> > On Wed, Jul 14, 2021 at 3:57 PM Till Rohrmann <tr...@apache.org>
> wrote:
> >> I think this suggestion makes a lot of sense, Stephan. +1 for fixing
> >> deprecation warnings when bumping/changing dependencies. I actually
> found
> >> myself recently, whenever touching a test class, replacing Junit's
> >> assertThat with Hamcrest's version which felt quite tedious.
> >>
> >> Cheers,
> >> Till
> >>
> >> On Tue, Jul 13, 2021 at 6:15 PM Stephan Ewen <se...@apache.org> wrote:
> >>
> >>> Hi all!
> >>>
> >>> I would like to propose that we make it a project standard that when
> >>> upgrading a dependency, deprecation issues arising from that need to be
> >>> fixed in the same step. If the new dependency version deprecates a
> method
> >>> in favor of another method, all usages in the code need to be replaced
> >>> together with the upgrade.
> >>>
> >>> We are accumulating deprecated API uses over time, and it floods logs
> and
> >>> IDEs with deprecation warnings. I find this is a problem, because the
> >>> irrelevant warnings more and more drown out the actually relevant
> warnings.
> >>> And arguably, the deprecation warning isn't fully irrelevant, it can
> cause
> >>> problems in the future when the method is actually removed.
> >>> We need the general principle that a change leaves the codebase in at
> least
> >>> as good shape as before, otherwise things accumulate over time and the
> >>> overall quality goes down.
> >>>
> >>> The concrete example that motivated this for me is the JUnit dependency
> >>> upgrade. Pretty much every test I looked at recently is quite yellow
> (due
> >>> to junit Matchers.assertThat being deprecated in the new JUnit
> version).
> >>> This is easily fixed (even a string replace and spotless:apply goes a
> long
> >>> way), so I would suggest we try and do these things in one step in the
> >>> future.
> >>>
> >>> Curious what other committers think about this suggestion.
> >>>
> >>> Best,
> >>> Stephan
> >>>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Chesnay Schepler <ch...@apache.org>.
How do you propose to do this in practice?
Let's say I bump jackson, how would I find all new usages of deprecated 
APIs?
Build it locally and grep the maven output for jackson?

On 14/07/2021 10:51, Yangze Guo wrote:
> +1 for fixing deprecation warnings when bumping/changing dependencies.
>
> Not only for the dependencies, we also use the deprecated API of Flink
> itself in `flink-examples` and the document, e.g. the #writeAsText. I
> think it would be good to do a clean-up for usability. WDYT?
>
> Best,
> Yangze Guo
>
> On Wed, Jul 14, 2021 at 3:57 PM Till Rohrmann <tr...@apache.org> wrote:
>> I think this suggestion makes a lot of sense, Stephan. +1 for fixing
>> deprecation warnings when bumping/changing dependencies. I actually found
>> myself recently, whenever touching a test class, replacing Junit's
>> assertThat with Hamcrest's version which felt quite tedious.
>>
>> Cheers,
>> Till
>>
>> On Tue, Jul 13, 2021 at 6:15 PM Stephan Ewen <se...@apache.org> wrote:
>>
>>> Hi all!
>>>
>>> I would like to propose that we make it a project standard that when
>>> upgrading a dependency, deprecation issues arising from that need to be
>>> fixed in the same step. If the new dependency version deprecates a method
>>> in favor of another method, all usages in the code need to be replaced
>>> together with the upgrade.
>>>
>>> We are accumulating deprecated API uses over time, and it floods logs and
>>> IDEs with deprecation warnings. I find this is a problem, because the
>>> irrelevant warnings more and more drown out the actually relevant warnings.
>>> And arguably, the deprecation warning isn't fully irrelevant, it can cause
>>> problems in the future when the method is actually removed.
>>> We need the general principle that a change leaves the codebase in at least
>>> as good shape as before, otherwise things accumulate over time and the
>>> overall quality goes down.
>>>
>>> The concrete example that motivated this for me is the JUnit dependency
>>> upgrade. Pretty much every test I looked at recently is quite yellow (due
>>> to junit Matchers.assertThat being deprecated in the new JUnit version).
>>> This is easily fixed (even a string replace and spotless:apply goes a long
>>> way), so I would suggest we try and do these things in one step in the
>>> future.
>>>
>>> Curious what other committers think about this suggestion.
>>>
>>> Best,
>>> Stephan
>>>


Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Yangze Guo <ka...@gmail.com>.
+1 for fixing deprecation warnings when bumping/changing dependencies.

Not only for the dependencies, we also use the deprecated API of Flink
itself in `flink-examples` and the document, e.g. the #writeAsText. I
think it would be good to do a clean-up for usability. WDYT?

Best,
Yangze Guo

On Wed, Jul 14, 2021 at 3:57 PM Till Rohrmann <tr...@apache.org> wrote:
>
> I think this suggestion makes a lot of sense, Stephan. +1 for fixing
> deprecation warnings when bumping/changing dependencies. I actually found
> myself recently, whenever touching a test class, replacing Junit's
> assertThat with Hamcrest's version which felt quite tedious.
>
> Cheers,
> Till
>
> On Tue, Jul 13, 2021 at 6:15 PM Stephan Ewen <se...@apache.org> wrote:
>
> > Hi all!
> >
> > I would like to propose that we make it a project standard that when
> > upgrading a dependency, deprecation issues arising from that need to be
> > fixed in the same step. If the new dependency version deprecates a method
> > in favor of another method, all usages in the code need to be replaced
> > together with the upgrade.
> >
> > We are accumulating deprecated API uses over time, and it floods logs and
> > IDEs with deprecation warnings. I find this is a problem, because the
> > irrelevant warnings more and more drown out the actually relevant warnings.
> > And arguably, the deprecation warning isn't fully irrelevant, it can cause
> > problems in the future when the method is actually removed.
> > We need the general principle that a change leaves the codebase in at least
> > as good shape as before, otherwise things accumulate over time and the
> > overall quality goes down.
> >
> > The concrete example that motivated this for me is the JUnit dependency
> > upgrade. Pretty much every test I looked at recently is quite yellow (due
> > to junit Matchers.assertThat being deprecated in the new JUnit version).
> > This is easily fixed (even a string replace and spotless:apply goes a long
> > way), so I would suggest we try and do these things in one step in the
> > future.
> >
> > Curious what other committers think about this suggestion.
> >
> > Best,
> > Stephan
> >

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Jingsong Li <ji...@gmail.com>.
BIG +1 for this. Thanks Stephan for starting this discussion.

Too many warnings in IDE and mvn build are frustrating. We can have the
opportunity to avoid the degradation of code quality.

Best,
Jingsong

On Thu, Jul 15, 2021 at 3:26 PM David Morávek <da...@gmail.com>
wrote:

> I know that sonar can report back by adding a comment to the issue (very
> similar way the FlinkBot does) and can block the merge (probably using
> check runs api [1]), if some quality gate fails. I was never setting it up,
> so I'd need to take a closer look. This is a feature set we were using on
> GH in the past.
>
> [1] https://docs.github.com/en/rest/reference/checks#runs
>
> D.
>
> On Thu, Jul 15, 2021 at 8:49 AM Robert Metzger <rm...@apache.org>
> wrote:
>
> > >
> > > Maybe we could leverage sonar cloud infrastructure for this. They
> already
> > > have built in rules for deprecation warnings [1]. Also they have a free
> > > offering for public open-source repositories [2].
> > >
> >
> > Cool, I didn't know this. There are already quite a lot of Apache
> Projects
> > there: https://sonarcloud.io/organizations/apache/projects, and INFRA
> > seems
> > to be very open to adding more projects:
> > https://issues.apache.org/jira/browse/INFRA-19555
> >
> > Do you know if GH PR integration is also available? (so that it shows a
> > warning in the PR if inspections on the change trigger)
> >
>


-- 
Best, Jingsong Lee

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by David Morávek <da...@gmail.com>.
I know that sonar can report back by adding a comment to the issue (very
similar way the FlinkBot does) and can block the merge (probably using
check runs api [1]), if some quality gate fails. I was never setting it up,
so I'd need to take a closer look. This is a feature set we were using on
GH in the past.

[1] https://docs.github.com/en/rest/reference/checks#runs

D.

On Thu, Jul 15, 2021 at 8:49 AM Robert Metzger <rm...@apache.org> wrote:

> >
> > Maybe we could leverage sonar cloud infrastructure for this. They already
> > have built in rules for deprecation warnings [1]. Also they have a free
> > offering for public open-source repositories [2].
> >
>
> Cool, I didn't know this. There are already quite a lot of Apache Projects
> there: https://sonarcloud.io/organizations/apache/projects, and INFRA
> seems
> to be very open to adding more projects:
> https://issues.apache.org/jira/browse/INFRA-19555
>
> Do you know if GH PR integration is also available? (so that it shows a
> warning in the PR if inspections on the change trigger)
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Robert Metzger <rm...@apache.org>.
>
> Maybe we could leverage sonar cloud infrastructure for this. They already
> have built in rules for deprecation warnings [1]. Also they have a free
> offering for public open-source repositories [2].
>

Cool, I didn't know this. There are already quite a lot of Apache Projects
there: https://sonarcloud.io/organizations/apache/projects, and INFRA seems
to be very open to adding more projects:
https://issues.apache.org/jira/browse/INFRA-19555

Do you know if GH PR integration is also available? (so that it shows a
warning in the PR if inspections on the change trigger)

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by David Morávek <dm...@apache.org>.
>
> For implementing this in practice, we could also extend our CI pipeline a
> bit, and count the number of deprecation warnings while compiling Flink.
> We would hard-code the current number of deprecations and fail the build if
> that number increases.


Maybe we could leverage sonar cloud infrastructure for this. They already
have built in rules for deprecation warnings [1]. Also they have a free
offering for public open-source repositories [2].

[1] https://rules.sonarsource.com/java/RSPEC-1874
[2] https://sonarcloud.io/pricing

On Wed, Jul 14, 2021 at 5:38 PM Chesnay Schepler <ch...@apache.org> wrote:

> If someone started preparing a junit5 migration PR they will run into
> merge conflicts if everyone now starts replacing these instances at will.
>
> There are also some options on the table on how to actually do the
> migration; we can use hamcrest of course, or create a small wrapper in
> our test utils that retains the signature junit signature (then we'd
> just have to adjust imports).
>
> On 14/07/2021 16:33, Stephan Ewen wrote:
> > @Chesnay - can you elaborate on this? In the classes I worked with so
> far,
> > it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
> > "org.hamcrest.MatcherAssert.assertThat()".
> > What other migration should happen there?
> >
> > On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
> > wrote:
> >
> >> It may be better to not do that to ease the migration to junit5, where
> >> we have to address exactly these usages.
> >>
> >> On 14/07/2021 09:57, Till Rohrmann wrote:
> >>> I actually found
> >>> myself recently, whenever touching a test class, replacing Junit's
> >>> assertThat with Hamcrest's version which felt quite tedious.
> >>
> >>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Martijn Visser <ma...@ververica.com>.
I was referring to the proposal to migrate all existing tests to JUnit 5
via one giant commit (as stated in step 3 from the voting email in the link
I sent)

On Wed, 14 Jul 2021 at 19:20, Chesnay Schepler <ch...@apache.org> wrote:

> I don't believe this case was *explicitly *addressed in either the vote
> or discussion thread. Please link the specific post if it was indeed
> mentioned.
>
> On 14/07/2021 19:13, Martijn Visser wrote:
>
> With regards to JUnit 5, there was a specific proposal and vote on how to
> deal with that migration [1]
>
> Best regards,
>
> Martijn
>
> [1]https://lists.apache.org/thread.html/r89a2675bce01ccfdcfc47f2b0af6ef1afdbe4bad96d8c679cf68825e%40%3Cdev.flink.apache.org%3E
>
>
>
> On Wed, 14 Jul 2021 at 17:31, Chesnay Schepler <ch...@apache.org> <ch...@apache.org> wrote:
>
>
> If someone started preparing a junit5 migration PR they will run into
> merge conflicts if everyone now starts replacing these instances at will.
>
> There are also some options on the table on how to actually do the
> migration; we can use hamcrest of course, or create a small wrapper in
> our test utils that retains the signature junit signature (then we'd
> just have to adjust imports).
>
> On 14/07/2021 16:33, Stephan Ewen wrote:
>
> @Chesnay - can you elaborate on this? In the classes I worked with so
>
> far,
>
> it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
> "org.hamcrest.MatcherAssert.assertThat()".
> What other migration should happen there?
>
> On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org> <ch...@apache.org>
> wrote:
>
>
> It may be better to not do that to ease the migration to junit5, where
> we have to address exactly these usages.
>
> On 14/07/2021 09:57, Till Rohrmann wrote:
>
> I actually found
> myself recently, whenever touching a test class, replacing Junit's
> assertThat with Hamcrest's version which felt quite tedious.
>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Chesnay Schepler <ch...@apache.org>.
I don't believe this case was /explicitly /addressed in either the vote 
or discussion thread. Please link the specific post if it was indeed 
mentioned.

On 14/07/2021 19:13, Martijn Visser wrote:
> With regards to JUnit 5, there was a specific proposal and vote on how to
> deal with that migration [1]
>
> Best regards,
>
> Martijn
>
> [1]
> https://lists.apache.org/thread.html/r89a2675bce01ccfdcfc47f2b0af6ef1afdbe4bad96d8c679cf68825e%40%3Cdev.flink.apache.org%3E
>
>
>
> On Wed, 14 Jul 2021 at 17:31, Chesnay Schepler <ch...@apache.org> wrote:
>
>> If someone started preparing a junit5 migration PR they will run into
>> merge conflicts if everyone now starts replacing these instances at will.
>>
>> There are also some options on the table on how to actually do the
>> migration; we can use hamcrest of course, or create a small wrapper in
>> our test utils that retains the signature junit signature (then we'd
>> just have to adjust imports).
>>
>> On 14/07/2021 16:33, Stephan Ewen wrote:
>>> @Chesnay - can you elaborate on this? In the classes I worked with so
>> far,
>>> it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
>>> "org.hamcrest.MatcherAssert.assertThat()".
>>> What other migration should happen there?
>>>
>>> On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
>>> wrote:
>>>
>>>> It may be better to not do that to ease the migration to junit5, where
>>>> we have to address exactly these usages.
>>>>
>>>> On 14/07/2021 09:57, Till Rohrmann wrote:
>>>>> I actually found
>>>>> myself recently, whenever touching a test class, replacing Junit's
>>>>> assertThat with Hamcrest's version which felt quite tedious.
>>>>
>>


Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Martijn Visser <ma...@ververica.com>.
With regards to JUnit 5, there was a specific proposal and vote on how to
deal with that migration [1]

Best regards,

Martijn

[1]
https://lists.apache.org/thread.html/r89a2675bce01ccfdcfc47f2b0af6ef1afdbe4bad96d8c679cf68825e%40%3Cdev.flink.apache.org%3E



On Wed, 14 Jul 2021 at 17:31, Chesnay Schepler <ch...@apache.org> wrote:

> If someone started preparing a junit5 migration PR they will run into
> merge conflicts if everyone now starts replacing these instances at will.
>
> There are also some options on the table on how to actually do the
> migration; we can use hamcrest of course, or create a small wrapper in
> our test utils that retains the signature junit signature (then we'd
> just have to adjust imports).
>
> On 14/07/2021 16:33, Stephan Ewen wrote:
> > @Chesnay - can you elaborate on this? In the classes I worked with so
> far,
> > it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
> > "org.hamcrest.MatcherAssert.assertThat()".
> > What other migration should happen there?
> >
> > On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
> > wrote:
> >
> >> It may be better to not do that to ease the migration to junit5, where
> >> we have to address exactly these usages.
> >>
> >> On 14/07/2021 09:57, Till Rohrmann wrote:
> >>> I actually found
> >>> myself recently, whenever touching a test class, replacing Junit's
> >>> assertThat with Hamcrest's version which felt quite tedious.
> >>
> >>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Stephan Ewen <se...@apache.org>.
>
> If someone started preparing a junit5 migration PR they will run into
> merge conflicts if everyone now starts replacing these instances at will.


This sounds like a good case for fixing it in one step during the upgrade.
Otherwise folks will start individually fixing this individually when they
encounter the deprecated methods.

On Wed, Jul 14, 2021 at 5:31 PM Chesnay Schepler <ch...@apache.org> wrote:

> If someone started preparing a junit5 migration PR they will run into
> merge conflicts if everyone now starts replacing these instances at will.
>
> There are also some options on the table on how to actually do the
> migration; we can use hamcrest of course, or create a small wrapper in
> our test utils that retains the signature junit signature (then we'd
> just have to adjust imports).
>
> On 14/07/2021 16:33, Stephan Ewen wrote:
> > @Chesnay - can you elaborate on this? In the classes I worked with so
> far,
> > it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
> > "org.hamcrest.MatcherAssert.assertThat()".
> > What other migration should happen there?
> >
> > On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
> > wrote:
> >
> >> It may be better to not do that to ease the migration to junit5, where
> >> we have to address exactly these usages.
> >>
> >> On 14/07/2021 09:57, Till Rohrmann wrote:
> >>> I actually found
> >>> myself recently, whenever touching a test class, replacing Junit's
> >>> assertThat with Hamcrest's version which felt quite tedious.
> >>
> >>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Chesnay Schepler <ch...@apache.org>.
If someone started preparing a junit5 migration PR they will run into 
merge conflicts if everyone now starts replacing these instances at will.

There are also some options on the table on how to actually do the 
migration; we can use hamcrest of course, or create a small wrapper in 
our test utils that retains the signature junit signature (then we'd 
just have to adjust imports).

On 14/07/2021 16:33, Stephan Ewen wrote:
> @Chesnay - can you elaborate on this? In the classes I worked with so far,
> it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
> "org.hamcrest.MatcherAssert.assertThat()".
> What other migration should happen there?
>
> On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
> wrote:
>
>> It may be better to not do that to ease the migration to junit5, where
>> we have to address exactly these usages.
>>
>> On 14/07/2021 09:57, Till Rohrmann wrote:
>>> I actually found
>>> myself recently, whenever touching a test class, replacing Junit's
>>> assertThat with Hamcrest's version which felt quite tedious.
>>
>>


Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Stephan Ewen <se...@apache.org>.
@Chesnay - can you elaborate on this? In the classes I worked with so far,
it was a 1:1 replacement of "org.junit.Assert.assertThat()" to
"org.hamcrest.MatcherAssert.assertThat()".
What other migration should happen there?

On Wed, Jul 14, 2021 at 11:13 AM Chesnay Schepler <ch...@apache.org>
wrote:

> It may be better to not do that to ease the migration to junit5, where
> we have to address exactly these usages.
>
> On 14/07/2021 09:57, Till Rohrmann wrote:
> > I actually found
> > myself recently, whenever touching a test class, replacing Junit's
> > assertThat with Hamcrest's version which felt quite tedious.
>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Robert Metzger <rm...@apache.org>.
For implementing this in practice, we could also extend our CI pipeline a
bit, and count the number of deprecation warnings while compiling Flink.
We would hard-code the current number of deprecations and fail the build if
that number increases.

We could actually extend this and run a curated list of IntelliJ
inspections during the build (IIRC this was discussed in the past):
https://www.jetbrains.com/help/idea/command-line-code-inspector.html

On Wed, Jul 14, 2021 at 11:14 AM Chesnay Schepler <ch...@apache.org>
wrote:

> It may be better to not do that to ease the migration to junit5, where
> we have to address exactly these usages.
>
> On 14/07/2021 09:57, Till Rohrmann wrote:
> > I actually found
> > myself recently, whenever touching a test class, replacing Junit's
> > assertThat with Hamcrest's version which felt quite tedious.
>
>
>

Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Chesnay Schepler <ch...@apache.org>.
It may be better to not do that to ease the migration to junit5, where 
we have to address exactly these usages.

On 14/07/2021 09:57, Till Rohrmann wrote:
> I actually found
> myself recently, whenever touching a test class, replacing Junit's
> assertThat with Hamcrest's version which felt quite tedious.



Re: [DISCUSS] Address deprecation warnings when upgrading dependencies

Posted by Till Rohrmann <tr...@apache.org>.
I think this suggestion makes a lot of sense, Stephan. +1 for fixing
deprecation warnings when bumping/changing dependencies. I actually found
myself recently, whenever touching a test class, replacing Junit's
assertThat with Hamcrest's version which felt quite tedious.

Cheers,
Till

On Tue, Jul 13, 2021 at 6:15 PM Stephan Ewen <se...@apache.org> wrote:

> Hi all!
>
> I would like to propose that we make it a project standard that when
> upgrading a dependency, deprecation issues arising from that need to be
> fixed in the same step. If the new dependency version deprecates a method
> in favor of another method, all usages in the code need to be replaced
> together with the upgrade.
>
> We are accumulating deprecated API uses over time, and it floods logs and
> IDEs with deprecation warnings. I find this is a problem, because the
> irrelevant warnings more and more drown out the actually relevant warnings.
> And arguably, the deprecation warning isn't fully irrelevant, it can cause
> problems in the future when the method is actually removed.
> We need the general principle that a change leaves the codebase in at least
> as good shape as before, otherwise things accumulate over time and the
> overall quality goes down.
>
> The concrete example that motivated this for me is the JUnit dependency
> upgrade. Pretty much every test I looked at recently is quite yellow (due
> to junit Matchers.assertThat being deprecated in the new JUnit version).
> This is easily fixed (even a string replace and spotless:apply goes a long
> way), so I would suggest we try and do these things in one step in the
> future.
>
> Curious what other committers think about this suggestion.
>
> Best,
> Stephan
>