-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new rule AG0041 for detecting and converting logs to templates #184
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using System; | ||
{testCase.Usings} | ||
|
||
namespace TestNamespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my 2 cents but I would optimize for readability instead of reduced code duplication in this case. I find the test cases a bit difficult to follow when I need to do string temp laying in my head.
Edit: or maybe just inlining all the using
s and privates into the template to reduce noise from the test cases might also help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I go the other way.
I love to use test cases. Just keep the test function itself really straight-forward and it should be basically:
"Setup", "Input", "expectedOutput" type of tests.
Then you can just read the TestDate and understand all the different tests cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree about test readability, i'll fix it up in another MR, there's 2 like this now, so I'll come up with a better solution and fix them all in one go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #184 +/- ##
==========================================
+ Coverage 74.20% 74.42% +0.22%
==========================================
Files 64 67 +3
Lines 2322 2428 +106
Branches 273 290 +17
==========================================
+ Hits 1723 1807 +84
- Misses 537 551 +14
- Partials 62 70 +8 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Szabó, Péter <[email protected]>
resolves #183