-
Notifications
You must be signed in to change notification settings - Fork 29
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
Split up utils module #138
Conversation
Rebased on latest master. |
Codecov Report
@@ Coverage Diff @@
## master #138 +/- ##
==========================================
- Coverage 33.51% 32.55% -0.97%
==========================================
Files 16 16
Lines 3932 3831 -101
==========================================
- Hits 1318 1247 -71
+ Misses 2614 2584 -30
Continue to review full report at Codecov.
|
1a47dfc
to
2a412a4
Compare
558f88e
to
dab5aec
Compare
This is ready for review now. |
echo "ERROR: doc wasn't build exiting" | ||
exit -1 | ||
fi | ||
# Checks if doctests were actually built - currently we don't have any though |
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.
Why?
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.
Ok, I've now read the comment.
I don't know if we should rather:
- remove doctests (seriously, we won't have any in the near to middle future)
- keep the code
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.
If you want, I can remove this code, but I'd rather keep it. That would make this coverage config more complete.
In general LGTM! |
That name came up naturally. Before, we just put all kind of utility functions in So I would like to distinguish between |
Hmm, for me |
Well we could also make this a module which only lives in the binary and not in the library ( |
bors r+ |
Binaries get a much smaller bin_utils and other helper functions are replaced/moved.
Canceled. |
bors r+ |
Timed out. |
#@$%! 🤬🤬🤬 bors r+ |
Build failed: |
bors retry |
Binaries get a much smaller bin_utils and other helper functions are
replaced/moved.