-
-
Notifications
You must be signed in to change notification settings - Fork 763
feat(fingreprinting): add .taskignore #1932
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?
Conversation
|
Greetings, and thanks for a wonderful project! The reasoning behind this PR is as follows: DesignSince the project already heavily relies on glob semantics, aligning the same semantics in the Taskfile's
|
9102660 to
bf471c7
Compare
| - public/bundle.css | ||
| ``` | ||
|
|
||
| It is also possible to exclude files from fingerprinting globally by creating `.taskignore` |
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.
nit: I would add "a" -- that is, change
[...] creating
.taskignorefile [...]
to
[...] creating a
.taskignorefile [...]
| @@ -0,0 +1 @@ | |||
| ./**/*.json | |||
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.
Suggest adding a comment line to make sure comments are correctly processed:
# ./dont_ignore.txt
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.
Also some blank lines to make sure trimming also works correctly.
|
|
||
| for _, line := range lines { | ||
| trimmed := strings.TrimSpace(line) | ||
| if trimmed != "" && !strings.HasPrefix(trimmed, "#") { |
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.
Users might expect that end-line comments (e.g., ./**/*.json # ignore pesky JSON files) are allowed, as they tend to be allowed in similar file formats.
adamdicarlo0
left a 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.
Looks pretty good to me.
If exclude supports negations, I would clarify in the docs and code whether negations are allowed in the .taskignore file (and unit test it).
I'm not sure whether they'd be supported, from looking at the code (I don't recall whether exclude supports them).
NB: I am not a maintainer or (at least not yet) even a contributor to the project!
|
It would be great to see this merged! |
|
Any plans for merging this? |
Fixes #1753