-
Notifications
You must be signed in to change notification settings - Fork 71
Refactor attribute with arguments fluent syntax methods #432
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
base: main
Are you sure you want to change the base?
Refactor attribute with arguments fluent syntax methods #432
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #432 +/- ##
==========================================
- Coverage 74.86% 73.71% -1.15%
==========================================
Files 257 258 +1
Lines 17221 16374 -847
Branches 1575 1338 -237
==========================================
- Hits 12892 12070 -822
- Misses 3873 3887 +14
+ Partials 456 417 -39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Alexander Linne <[email protected]>
38d7e45 to
2edbb5a
Compare
| @@ -0,0 +1,48 @@ | |||
| using System; | |||
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.
nitpick: using System; is unnecessary
| { | ||
| var attArguments = instance | ||
| .GetAllNamedAttributeArgumentTuples() | ||
| .ToList(); |
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 noticed you are using .ToList() in many places in this file where it would not be necessary. Maybe it would be worth it to have a second look as leaving out the unnecessary conversions into lists could increase the performance massively.
| return $"{instance.Type.FullName} {argumentsDescription}"; | ||
| }); | ||
| var failDescription = argumentsDescriptions.FormatDescription( | ||
| "does not have any attribute", |
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.
nitpick: Missing 's' at the end - "does not have any attributes"
Attribute(s)With(Named)Argumentsfluent syntax methods and reduce code duplication by refactoring common logic into helper methods.AttributesWithArgumentswe only keep theIEnumerable<object>overload and remove theparams object[]overload to remove the type ambiguity.