-
Notifications
You must be signed in to change notification settings - Fork 3
add explicit derivatives of KL shell #225
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
Conversation
@tarun-mitruka @henrij22 as an exercise can you give me a code Review. Suggest changes, more comments/doxygen comments or unclear implementations or other suggestions for refactoring. You can wait till the CI worked |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #225 +/- ##
===========================================
+ Coverage 82.53% 92.87% +10.33%
===========================================
Files 48 49 +1
Lines 1867 2022 +155
===========================================
+ Hits 1541 1878 +337
+ Misses 326 144 -182
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
c1eced6
to
82af910
Compare
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.
Hey Alex, thanks for the PR and giving for the PR review exercises. I added some comments. May be some can be incorporated in this PR and some can be moved to a new PR by opening a new issue.
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.
thanks for letting me review this PR.
I just found a few minor things, you could think about changing
1565365
to
c44f040
Compare
9b08563
to
7d148de
Compare
7d148de
to
dc4ffaf
Compare
dc4ffaf
to
0dfe755
Compare
Closes #88.