New Rule: Property Name's Should be PascalCase

Suchiman / SerilogAnalyzer

Roslyn-based analysis for code using the Serilog logging library. Checks for common mistakes and usage problems.

Apache License 2.0

309 stars 29 forks source link

New Rule: Property Name's Should be PascalCase #16

Closed normanhh3 closed 7 years ago

normanhh3 commented 7 years ago

Hi Folks,

I'm currently using Serilog and love it. My team is new to the tool and I'm looking for ways to ensure logging standards and compliance. This one looks like a great start.

We have settled on a standard of property names are in CamelCase.

Is this something that fits within the scheme of constraints you envision for this tool?

Suchiman commented 7 years ago

I think this is a great idea! Although i think you mean PascalCase instead of camelCase I'm wondering if i can make this configurable since style is always a highly discussed topic 😄 .

nblumhardt commented 7 years ago

All of the Serilog examples use PascalCase too 👍 - I've seen other schemes used in the wild though. For some, it's a matter of matching the log format from other systems that might be written in e.g. JavaScript, where norms are different.

normanhh3 commented 7 years ago

Yes, apparently I had a moment there.

It might best be something done via opt-in configuration.

On Mar 30, 2017 4:14 PM, "Nicholas Blumhardt" notifications@github.com wrote:

All of the Serilog examples use PascalCase too 👍 - I've seen other schemes used in the wild though. For some, it's a matter of matching the log format from other systems that might be written in e.g. JavaScript, where norms are different.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Suchiman/SerilogAnalyzer/issues/16#issuecomment-290531357, or mute the thread https://github.com/notifications/unsubscribe-auth/ABj7xDAC1TvAC3Z7mIVSRKuXrquwG7MTks5rrA0qgaJpZM4MuvDl .

MrKevHunter commented 7 years ago

I've made a start on this here

https://github.com/Suchiman/SerilogAnalyzer/pull/21

There is a passing test and the analyser is written, need to handle configuration, I've made it a warning for now

Suchiman commented 7 years ago

Yeah the "configuration" part is tricky...

If you have the analyzer installed as a nuget package, you can set its severity through a .ruleset file.
If you have the vsix installed, you have no such option. Instead you could go deeper into VS to make a property page in the options...
Simpler would be a "Magic file" that contains settings and gets read by the analyzer manually but that wouldn't be well behaved and probably violates the principle of least astonishment.

MrKevHunter commented 7 years ago

Ok, the analyzer and code fix is done and is in the PR, just need to work out how to do config

normanhh3 commented 7 years ago

Wow that is great!

Suchiman commented 7 years ago

Thanks to @MrKevHunter this is fixed with #21 👍 Configurability is currently only given through the default configuration options you have so far:

You can go into the project settings and set "suppress warnings: Serilog006" which disables the warning in the build and IntelliSense
You can use a ruleset file to change the severity of the analyzer.