-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/BookApplication #456
Feat/BookApplication #456
Conversation
2b88374
to
1624816
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## development #456 +/- ##
===============================================
+ Coverage 77.04% 77.74% +0.69%
===============================================
Files 310 323 +13
Lines 6980 7464 +484
Branches 713 754 +41
===============================================
+ Hits 5378 5803 +425
- Misses 1394 1436 +42
- Partials 208 225 +17
☔ View full report in Codecov by Sentry. |
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.
Good PR overall, some minor change. Could you please fix Code Smell and bring code coverage at 70% ? Thanks
@typeparam TItem | ||
|
||
@{ | ||
var columnConditionalClass = this.IsCollapsed ? "collapsed" : ""; |
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.
Can you put this as private properties, inside the cs file?
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.
Those are indeed properties of the component. However adding variables for the conditional classes is preferable in the razor component. It's easier to debug.
Kudos, SonarCloud Quality Gate passed! 0 Bugs 72.0% Coverage The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. |
Prerequisites
Description
This PR implements the BookEditor application