-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
CI simplify check-watermark #4200
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
CI simplify check-watermark #4200
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4200 +/- ##
=======================================
Coverage 88.91% 88.91%
=======================================
Files 89 89
Lines 14429 14429
=======================================
Hits 12829 12829
Misses 1600 1600 |
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.
Super nice, thanks @MarcoGorelli !
Just one thing before merging: can you add the minimal pre-commit version requirement to environment.yml
too please? It's useful for people using conda 😉
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 have zero experience with pre-commit
, so take my approval with a grain of salt. Just left one comment :)
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.
All good now, thanks @MarcoGorelli !
I got a PR in to pre-commit to add a
--negate
flag topygrep
, usingcheck-watermark
as an example use-case 🎉So...let's use it? It means not having to maintain an extra
scripts/check_watermark.py
file, which can only be a good thingpygrep
is a Python implementation of grep, so is cross-platform--negate
will exit with code 1 if a pattern is not found (in this case, we want the watermark to be found in every notebook)--multiline
makes pygrep look through the entire file, rather than just line-by-line