audaciaconsulting / Audacia.CodeAnalysis

MIT License
1 stars 3 forks source link

Add analysers that support our logging standards #33

Open Audacia-RhysSmith opened 1 month ago

Audacia-RhysSmith commented 1 month ago
  1. If log messages contains an interpolated string, should use template message (FormatableString in C#)
  2. Every command or service injects ILogger
  3. Every command should have an LogInformation that starts with Entry and starts with Exit
  4. Every log message should have more 3 words? (thinking behind is that 3 words isnt descriptive enough)
  5. [Removed] Violation for using _logger.Log with signature of just string, there is another signature which has the log level passed which is fine
  6. Check for duplicate log statements
  7. Violation for if the amount of log statements if greater than the amount of "functionality" lines. If there was 5 lines in a method, should there be 8 lines of logging (thinking is this would violate clear and concise logging)
richardb355 commented 1 month ago

My two cents on each one:

jackpercy-acl commented 3 weeks ago

My input on each one too:

OwenLacey28 commented 3 weeks ago

If log messages contains an interpolated string, should use template message (FormatableString in C#)

🟢 worth a very quick PR into Audacia.CodeAnalysis? Maybe at the same time as the logging standards are merged?

Every command or service injects ILogger

🟡 less convinced by this, do we want all of our commands/services to inject a logger?

Every log message should have more 3 words? (thinking behind is that 3 words isnt descriptive enough)

🟢 could be a char check - but agree with this in principle

Check for duplicate log statements

🟡 within the same method / class? For me this isn't a common enough issue, and doesn't catch scenarios like when copy/pasting from another area

Violation for if the amount of log statements if greater than the amount of "functionality" lines. If there was 5 lines in a method, should there be 8 lines of logging (thinking is this would violate clear and concise logging)

🟢 yeah why not

Audacia-RhysSmith commented 3 weeks ago

Created a PR to propose changes for points 1 and 2 - would appreciate any feedback

https://github.com/audaciaconsulting/Audacia.CodeAnalysis/pull/36