Getting warnings out

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Getting warnings out

Erick Erickson
I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.

On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.

OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.

So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.

The number of raw NamedList member variables we have is overwhelming all by itself….

Comments?


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

david.w.smiley@gmail.com
Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.

On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.

On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.

OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.

So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.

The number of raw NamedList member variables we have is overwhelming all by itself….

Comments?


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Erick Erickson
Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:

protected NamedList<Object> values = new SimpleOrderedMap<>();

but then declare a method:

public NamedList getValues() { return values; }

This is just bad practice.

I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.

It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.

Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.



> On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
>
> Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
>
> ~ David
>
>
> On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
>
> On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
>
> OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
>
> So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
>
> The number of raw NamedList member variables we have is overwhelming all by itself….
>
> Comments?
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Atri Sharma-3
+1 to Erick’s proposal.

I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.

Atri 

On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:

protected NamedList<Object> values = new SimpleOrderedMap<>();

but then declare a method:

public NamedList getValues() { return values; }

This is just bad practice.

I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.

It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.

Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.



> On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
>
> Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
>
> ~ David
>
>
> On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
>
> On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
>
> OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
>
> So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
>
> The number of raw NamedList member variables we have is overwhelming all by itself….
>
> Comments?
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

--
Regards,

Atri
Apache Concerted
Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Gus Heck
I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij...  

On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
+1 to Erick’s proposal.

I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.

Atri 

On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:

protected NamedList<Object> values = new SimpleOrderedMap<>();

but then declare a method:

public NamedList getValues() { return values; }

This is just bad practice.

I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.

It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.

Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.



> On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
>
> Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
>
> ~ David
>
>
> On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
>
> On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
>
> OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
>
> So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
>
> The number of raw NamedList member variables we have is overwhelming all by itself….
>
> Comments?
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

--
Regards,

Atri
Apache Concerted


--
Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Erick Erickson
Gus:

When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.

There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.

Best,
Erick

> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>
> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij...  
>
> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> +1 to Erick’s proposal.
>
> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>
> Atri
>
> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>
> protected NamedList<Object> values = new SimpleOrderedMap<>();
>
> but then declare a method:
>
> public NamedList getValues() { return values; }
>
> This is just bad practice.
>
> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>
> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>
> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>
>
>
> > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> >
> > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> >
> > ~ David
> >
> >
> > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> >
> > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> >
> > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> >
> > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> >
> > The number of raw NamedList member variables we have is overwhelming all by itself….
> >
> > Comments?
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Andras Salamon
Hi,

We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.


I have two jiras in patch-available status:


Andras

On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
Gus:

When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.

There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.

Best,
Erick

> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>
> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij... 
>
> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> +1 to Erick’s proposal.
>
> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>
> Atri
>
> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>
> protected NamedList<Object> values = new SimpleOrderedMap<>();
>
> but then declare a method:
>
> public NamedList getValues() { return values; }
>
> This is just bad practice.
>
> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>
> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>
> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>
>
>
> > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> >
> > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> >
> > ~ David
> >
> >
> > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> >
> > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> >
> > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> >
> > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> >
> > The number of raw NamedList member variables we have is overwhelming all by itself….
> >
> > Comments?
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Mike Drob-3
I agree with the one warning at a time approach. That one seems most feasible to take piecemeal

On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
Hi,

We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.


I have two jiras in patch-available status:


Andras

On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
Gus:

When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.

There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.

Best,
Erick

> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>
> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij... 
>
> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> +1 to Erick’s proposal.
>
> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>
> Atri
>
> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>
> protected NamedList<Object> values = new SimpleOrderedMap<>();
>
> but then declare a method:
>
> public NamedList getValues() { return values; }
>
> This is just bad practice.
>
> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>
> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>
> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>
>
>
> > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> >
> > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> >
> > ~ David
> >
> >
> > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> >
> > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> >
> > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> >
> > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> >
> > The number of raw NamedList member variables we have is overwhelming all by itself….
> >
> > Comments?
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

david.w.smiley@gmail.com
My proposal to disregard entire classes of warnings was intended to be a short term strategy and not long term.  It allows you to fix some problems completely  before moving onto others.  Additionally, we can tweak the linter settings per-module.  The final end state in the future is not to have such linter customizations.

~ David


On Mon, May 11, 2020 at 11:22 AM Mike Drob <[hidden email]> wrote:
I agree with the one warning at a time approach. That one seems most feasible to take piecemeal

On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
Hi,

We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.


I have two jiras in patch-available status:


Andras

On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
Gus:

When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.

There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.

Best,
Erick

> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>
> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij... 
>
> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> +1 to Erick’s proposal.
>
> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>
> Atri
>
> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>
> protected NamedList<Object> values = new SimpleOrderedMap<>();
>
> but then declare a method:
>
> public NamedList getValues() { return values; }
>
> This is just bad practice.
>
> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>
> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>
> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>
>
>
> > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> >
> > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> >
> > ~ David
> >
> >
> > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> >
> > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> >
> > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> >
> > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> >
> > The number of raw NamedList member variables we have is overwhelming all by itself….
> >
> > Comments?
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Gus Heck
@Erick assign me a (not too large) dir... I'll help.

On Mon, May 11, 2020 at 11:33 AM David Smiley <[hidden email]> wrote:
My proposal to disregard entire classes of warnings was intended to be a short term strategy and not long term.  It allows you to fix some problems completely  before moving onto others.  Additionally, we can tweak the linter settings per-module.  The final end state in the future is not to have such linter customizations.

~ David


On Mon, May 11, 2020 at 11:22 AM Mike Drob <[hidden email]> wrote:
I agree with the one warning at a time approach. That one seems most feasible to take piecemeal

On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
Hi,

We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.


I have two jiras in patch-available status:


Andras

On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
Gus:

When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.

There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.

Best,
Erick

> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>
> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij... 
>
> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> +1 to Erick’s proposal.
>
> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>
> Atri
>
> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>
> protected NamedList<Object> values = new SimpleOrderedMap<>();
>
> but then declare a method:
>
> public NamedList getValues() { return values; }
>
> This is just bad practice.
>
> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>
> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>
> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>
>
>
> > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> >
> > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> >
> > ~ David
> >
> >
> > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> >
> > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> >
> > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> >
> > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> >
> > The number of raw NamedList member variables we have is overwhelming all by itself….
> >
> > Comments?
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



--
Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Erick Erickson
Andras:

Yep, I saw a couple of those, I’m looking at how to incorporate. It may take another week or so, depending… Apologies for them sitting on the shelf for so long.



David:  Noted.

Mike:

Well, I’ve got some ready to look at more closely, I often tear into something for a while mostly to get a better feel for what it’ll take. I’ll probably post a PR for SOLR-10810 soon with what I’ve done so far, mainly getting auxiliary class warnings out of solr/core (which was something of a chore). Then on to Andras’ patches.

Gus:

The chunks are fairly big when it’s based on project. For instance solr/core or solrj/core or… I’m trying to get a clue how to break them up in to smaller chunks, directory-based would be best I think.

So here’s what I propose (I’ll take care of these steps):

1> get the PR in place for what I've done so far for SOLR-10810 (warning, it’s bigger than it should be, in future I need to take smaller bites) and push that upstream when we’re satisfied.

2> incorporate the changes Andras mentioned.

3> start the process of suppressing warnings. I propose that we make a list of subtasks from SOLR-10778 and assign them to whoever takes them on. If you can’t assign something to yourself, raise a subtask and make a note in the description that you’re working on XYZ.

Again, the short-term goal is to get clean compilations, fixing safe/simple warnings that we fully understand and SuppressWarnings for the rest. From there we’ll start failing on warnings when a project is clean.

Thanks!


> On May 11, 2020, at 11:40 AM, Gus Heck <[hidden email]> wrote:
>
> @Erick assign me a (not too large) dir... I'll help.
>
> On Mon, May 11, 2020 at 11:33 AM David Smiley <[hidden email]> wrote:
> My proposal to disregard entire classes of warnings was intended to be a short term strategy and not long term.  It allows you to fix some problems completely  before moving onto others.  Additionally, we can tweak the linter settings per-module.  The final end state in the future is not to have such linter customizations.
>
> ~ David
>
>
> On Mon, May 11, 2020 at 11:22 AM Mike Drob <[hidden email]> wrote:
> I agree with the one warning at a time approach. That one seems most feasible to take piecemeal
>
> On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
> Hi,
>
> We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.
>
> There are two umbrella jiras for that: https://issues.apache.org/jira/browse/SOLR-10778 and https://issues.apache.org/jira/browse/LUCENE-7907
>
> I have two jiras in patch-available status:
>
> https://issues.apache.org/jira/browse/LUCENE-9323
> https://issues.apache.org/jira/browse/SOLR-14266
>
> Andras
>
> On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
> Gus:
>
> When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.
>
> There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.
>
> Best,
> Erick
>
> > On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
> >
> > I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij...  
> >
> > On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> > +1 to Erick’s proposal.
> >
> > I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
> >
> > Atri
> >
> > On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> > Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
> >
> > protected NamedList<Object> values = new SimpleOrderedMap<>();
> >
> > but then declare a method:
> >
> > public NamedList getValues() { return values; }
> >
> > This is just bad practice.
> >
> > I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
> >
> > It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
> >
> > Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
> >
> >
> >
> > > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> > >
> > > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> > >
> > > ~ David
> > >
> > >
> > > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> > >
> > > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> > >
> > > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> > >
> > > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> > >
> > > The number of raw NamedList member variables we have is overwhelming all by itself….
> > >
> > > Comments?
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> > --
> > Regards,
> >
> > Atri
> > Apache Concerted
> >
> >
> > --
> > http://www.needhamsoftware.com (work)
> > http://www.the111shift.com (play)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
>
> --
> http://www.needhamsoftware.com (work)
> http://www.the111shift.com (play)


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Atri Sharma-3
Erick,

Please assign me a medium sized directory as well (in the Lucene JIRA).

Atri

On Mon, May 11, 2020 at 11:21 PM Erick Erickson <[hidden email]> wrote:

>
> Andras:
>
> Yep, I saw a couple of those, I’m looking at how to incorporate. It may take another week or so, depending… Apologies for them sitting on the shelf for so long.
>
>
>
> David:  Noted.
>
> Mike:
>
> Well, I’ve got some ready to look at more closely, I often tear into something for a while mostly to get a better feel for what it’ll take. I’ll probably post a PR for SOLR-10810 soon with what I’ve done so far, mainly getting auxiliary class warnings out of solr/core (which was something of a chore). Then on to Andras’ patches.
>
> Gus:
>
> The chunks are fairly big when it’s based on project. For instance solr/core or solrj/core or… I’m trying to get a clue how to break them up in to smaller chunks, directory-based would be best I think.
>
> So here’s what I propose (I’ll take care of these steps):
>
> 1> get the PR in place for what I've done so far for SOLR-10810 (warning, it’s bigger than it should be, in future I need to take smaller bites) and push that upstream when we’re satisfied.
>
> 2> incorporate the changes Andras mentioned.
>
> 3> start the process of suppressing warnings. I propose that we make a list of subtasks from SOLR-10778 and assign them to whoever takes them on. If you can’t assign something to yourself, raise a subtask and make a note in the description that you’re working on XYZ.
>
> Again, the short-term goal is to get clean compilations, fixing safe/simple warnings that we fully understand and SuppressWarnings for the rest. From there we’ll start failing on warnings when a project is clean.
>
> Thanks!
>
>
> > On May 11, 2020, at 11:40 AM, Gus Heck <[hidden email]> wrote:
> >
> > @Erick assign me a (not too large) dir... I'll help.
> >
> > On Mon, May 11, 2020 at 11:33 AM David Smiley <[hidden email]> wrote:
> > My proposal to disregard entire classes of warnings was intended to be a short term strategy and not long term.  It allows you to fix some problems completely  before moving onto others.  Additionally, we can tweak the linter settings per-module.  The final end state in the future is not to have such linter customizations.
> >
> > ~ David
> >
> >
> > On Mon, May 11, 2020 at 11:22 AM Mike Drob <[hidden email]> wrote:
> > I agree with the one warning at a time approach. That one seems most feasible to take piecemeal
> >
> > On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
> > Hi,
> >
> > We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.
> >
> > There are two umbrella jiras for that: https://issues.apache.org/jira/browse/SOLR-10778 and https://issues.apache.org/jira/browse/LUCENE-7907
> >
> > I have two jiras in patch-available status:
> >
> > https://issues.apache.org/jira/browse/LUCENE-9323
> > https://issues.apache.org/jira/browse/SOLR-14266
> >
> > Andras
> >
> > On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
> > Gus:
> >
> > When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.
> >
> > There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.
> >
> > Best,
> > Erick
> >
> > > On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
> > >
> > > I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij...
> > >
> > > On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
> > > +1 to Erick’s proposal.
> > >
> > > I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
> > >
> > > Atri
> > >
> > > On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
> > > Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
> > >
> > > protected NamedList<Object> values = new SimpleOrderedMap<>();
> > >
> > > but then declare a method:
> > >
> > > public NamedList getValues() { return values; }
> > >
> > > This is just bad practice.
> > >
> > > I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
> > >
> > > It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
> > >
> > > Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
> > >
> > >
> > >
> > > > On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
> > > >
> > > > Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
> > > > https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
> > > >
> > > > ~ David
> > > >
> > > >
> > > > On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
> > > > I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
> > > >
> > > > On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
> > > >
> > > > OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
> > > >
> > > > So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
> > > >
> > > > The number of raw NamedList member variables we have is overwhelming all by itself….
> > > >
> > > > Comments?
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > > --
> > > Regards,
> > >
> > > Atri
> > > Apache Concerted
> > >
> > >
> > > --
> > > http://www.needhamsoftware.com (work)
> > > http://www.the111shift.com (play)
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >
> >
> > --
> > http://www.needhamsoftware.com (work)
> > http://www.the111shift.com (play)
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


--
Regards,

Atri
Apache Concerted

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Getting warnings out

Erick Erickson
OK, I just put up a PR for SOLR-10810 with what I have so far for solr/core. I suspect it overlaps with some of the changes from the JIRAs people have pointed out. Please feel free to comment.

Meanwhile I’ll start looking at the other JIRAs and (eventually) reconcile them.

In case I messed up the PR, here’s the URL: https://github.com/apache/lucene-solr/pull/1509

Erick

> On May 11, 2020, at 1:54 PM, Atri Sharma <[hidden email]> wrote:
>
> Erick,
>
> Please assign me a medium sized directory as well (in the Lucene JIRA).
>
> Atri
>
> On Mon, May 11, 2020 at 11:21 PM Erick Erickson <[hidden email]> wrote:
>>
>> Andras:
>>
>> Yep, I saw a couple of those, I’m looking at how to incorporate. It may take another week or so, depending… Apologies for them sitting on the shelf for so long.
>>
>>
>>
>> David:  Noted.
>>
>> Mike:
>>
>> Well, I’ve got some ready to look at more closely, I often tear into something for a while mostly to get a better feel for what it’ll take. I’ll probably post a PR for SOLR-10810 soon with what I’ve done so far, mainly getting auxiliary class warnings out of solr/core (which was something of a chore). Then on to Andras’ patches.
>>
>> Gus:
>>
>> The chunks are fairly big when it’s based on project. For instance solr/core or solrj/core or… I’m trying to get a clue how to break them up in to smaller chunks, directory-based would be best I think.
>>
>> So here’s what I propose (I’ll take care of these steps):
>>
>> 1> get the PR in place for what I've done so far for SOLR-10810 (warning, it’s bigger than it should be, in future I need to take smaller bites) and push that upstream when we’re satisfied.
>>
>> 2> incorporate the changes Andras mentioned.
>>
>> 3> start the process of suppressing warnings. I propose that we make a list of subtasks from SOLR-10778 and assign them to whoever takes them on. If you can’t assign something to yourself, raise a subtask and make a note in the description that you’re working on XYZ.
>>
>> Again, the short-term goal is to get clean compilations, fixing safe/simple warnings that we fully understand and SuppressWarnings for the rest. From there we’ll start failing on warnings when a project is clean.
>>
>> Thanks!
>>
>>
>>> On May 11, 2020, at 11:40 AM, Gus Heck <[hidden email]> wrote:
>>>
>>> @Erick assign me a (not too large) dir... I'll help.
>>>
>>> On Mon, May 11, 2020 at 11:33 AM David Smiley <[hidden email]> wrote:
>>> My proposal to disregard entire classes of warnings was intended to be a short term strategy and not long term.  It allows you to fix some problems completely  before moving onto others.  Additionally, we can tweak the linter settings per-module.  The final end state in the future is not to have such linter customizations.
>>>
>>> ~ David
>>>
>>>
>>> On Mon, May 11, 2020 at 11:22 AM Mike Drob <[hidden email]> wrote:
>>> I agree with the one warning at a time approach. That one seems most feasible to take piecemeal
>>>
>>> On Mon, May 11, 2020 at 10:19 AM Andras Salamon <[hidden email]> wrote:
>>> Hi,
>>>
>>> We have quite a few warnings, it would be difficult to fix them at once. Checking one directory (or warning type) and handling 10-20 warnings at the same time seems more reasonable.
>>>
>>> There are two umbrella jiras for that: https://issues.apache.org/jira/browse/SOLR-10778 and https://issues.apache.org/jira/browse/LUCENE-7907
>>>
>>> I have two jiras in patch-available status:
>>>
>>> https://issues.apache.org/jira/browse/LUCENE-9323
>>> https://issues.apache.org/jira/browse/SOLR-14266
>>>
>>> Andras
>>>
>>> On Mon, May 11, 2020 at 4:28 PM Erick Erickson <[hidden email]> wrote:
>>> Gus:
>>>
>>> When it comes to actually removing the necessity of suppresswarnings IntelliJ makes a lot of this much easier. The issue is that it’s too much work for any one person to have a hope of doing in any reasonable period without introducing errors.
>>>
>>> There are just too many warnings for one person to have a hope of thinking carefully about all of them, so my strategy is to stop adding to the problem, raise awareness when it happens etc. I think to remove the necessity for SuppressWarnings will require extensive work, best approached over time, not in a huge wodge.
>>>
>>> Best,
>>> Erick
>>>
>>>> On May 11, 2020, at 9:25 AM, Gus Heck <[hidden email]> wrote:
>>>>
>>>> I typically battle warnings by conquering one file/directory at a time... And letting Intellij take me from warning to warning with F2 key really really really speeds things up. This is a wider set than compiler warnings, but you can customize it, and many of the additional warnings are auto-solvable (things like redundant initializers for variables that are already assigned before use), so the extra work is more than paid for by the reduction in transition time. The key one to think carefully about is the one that wants to minimize access, which is great for new classes, dangerous for released classes. Perhaps turn that warning off in intellij...
>>>>
>>>> On Mon, May 11, 2020 at 8:14 AM Atri Sharma <[hidden email]> wrote:
>>>> +1 to Erick’s proposal.
>>>>
>>>> I hate the number of warnings we get — we should still be formulating some sort of a strategy to fix them.
>>>>
>>>> Atri
>>>>
>>>> On Mon, 11 May 2020 at 17:09, Erick Erickson <[hidden email]> wrote:
>>>> Just disabling the warning globally nothing to prevent more being added. Take raw types. They’re a compromise allowed by the java compiler explicitly to be able to continue to use older binaries written before (or without) generics. But take a look at SolrQueryResponse for instance. We explicitly declare:
>>>>
>>>> protected NamedList<Object> values = new SimpleOrderedMap<>();
>>>>
>>>> but then declare a method:
>>>>
>>>> public NamedList getValues() { return values; }
>>>>
>>>> This is just bad practice.
>>>>
>>>> I don’t mind the grunt work, keeps me from stupid surfing. I’m proposing that I fix what’s easy, and suppress the rest.
>>>>
>>>> It might have been clearer if I’d said “Then start failing builds on any new warnings of these types”.
>>>>
>>>> Oh, and I’m also thinking of changing my BadApple report to flag when new SuppressWarnings are introduced and then nag people about new ones.
>>>>
>>>>
>>>>
>>>>> On May 10, 2020, at 11:43 PM, David Smiley <[hidden email]> wrote:
>>>>>
>>>>> Can't we customize the linting to disregard entire categories of certain warnings for now?  This makes your task manageable.
>>>>> https://discuss.gradle.org/t/recompile-with-xlint-parameters/25279
>>>>>
>>>>> ~ David
>>>>>
>>>>>
>>>>> On Sun, May 10, 2020 at 10:41 PM Erick Erickson <[hidden email]> wrote:
>>>>> I’m really struggling with what to do with compiler warnings, particularly all the rawtypes and unchecked warnings.
>>>>>
>>>>> On the one hand, the simple mechanical thing to do would be to SuppressWarnings on each one that exists presently. Frankly that feels pretty useless; that would preserve poor code forever.
>>>>>
>>>>> OTOH, actually _fixing_ the issues to not have, say, rawtypes is going to be time consuming and error-prone. Especially since I don’t really understand all the nuances yet and learning them one by one will introduce serious errors without doubt.
>>>>>
>>>>> So here’s what I propose. Even though it feels useless, just SuppressWarnings on anything that’s not a simple fix. Then start failing builds on these warnings to catch any that come in in future. At least that way there’ll be some incentive to keep the code from getting _worse_, although people will still be able to just add SuppressWarnings to the mix I suppose.
>>>>>
>>>>> The number of raw NamedList member variables we have is overwhelming all by itself….
>>>>>
>>>>> Comments?
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: [hidden email]
>>>>> For additional commands, e-mail: [hidden email]
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [hidden email]
>>>> For additional commands, e-mail: [hidden email]
>>>>
>>>> --
>>>> Regards,
>>>>
>>>> Atri
>>>> Apache Concerted
>>>>
>>>>
>>>> --
>>>> http://www.needhamsoftware.com (work)
>>>> http://www.the111shift.com (play)
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>>
>>>
>>> --
>>> http://www.needhamsoftware.com (work)
>>> http://www.the111shift.com (play)
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>
>
> --
> Regards,
>
> Atri
> Apache Concerted
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]