search

Lombiq / .NET-Analyzers

.NET code analyzers and code convention settings for Lombiq projects.
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

OSOE-760: Disable SA1518 since it's already covered by S113 #96

Closed 0liver closed 6 months ago

0liver commented 6 months ago
Piedone commented 6 months ago

Thanks for the contrib! Don't worry about the missing parent PR failing check, I'll add one.

Any particular reason why you didn't choose to keep the StyleCop rule instead?

0liver commented 6 months ago

Thanks! I had already forgotten that a parent PR is needed 😉

As for the choice of rule to keep - I think I liked the warning message better from S113, but looking at the great rules provided by StyleCop, keeping SA1518 instead seems like a good idea, too. Would you like me to switch them around?

Piedone commented 6 months ago

No, it's OK then. The StyleCop rule also requires some configuration so that makes things easier. Interestingly enough, we didn't actually add any config but it was still working, perhaps by some coincidence.

Piedone commented 6 months ago

Thank you!

0liver commented 6 months ago

It's been a pleasure! 😊

0liver commented 6 months ago

There's another duplicate, CA1711 and S2344:

image

Which one would you remove?

Piedone commented 6 months ago

I'd need to dig into that, but the SDK one is usually a safer bet.

0liver commented 6 months ago

the SDK one is usually a safer bet.

To keep, I suppose?

I just discovered why picking S113 over SA1518 might have been the wrong choice - there's no quick-fix action available for the former, but there is for the latter, which is greatly appreciated when you want to clean up your solution one warning at a time 😉

Do you fancy another PR to invert the exclusion?

Piedone commented 6 months ago

Yes, and sure, thanks.

0liver commented 6 months ago

Here's the follow-up PR: https://github.com/Lombiq/.NET-Analyzers/pull/97.