| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@nodejs/linting @nodejs/build-files @DerekNonGeneric |
Sorry, something went wrong.
|
On my laptop, this change saves 17 seconds on the markdown lint run when a single .md file in doc/api is edited. $ touch doc/api/assert.md && time make lint-md
Running Markdown linter on docs...
real 0m17.836s
user 0m20.322s
sys 0m1.128s
$ git stash pop && !!
git stash pop && touch doc/api/assert.md && time make lint-md
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: Makefile
no changes added to commit (use "git add" and/or "git commit -a")
Dropped refs/stash@{0} (b0765bc82f1d2948f958ebd630363f8dc67d2271)
Running Markdown linter on docs...
real 0m0.812s
user 0m0.601s
sys 0m0.218s
$ |
Sorry, something went wrong.
Sorry, something went wrong.
|
I agree that, due to the potential for a rebuilt linter to have different verification criteria, all verifiable files would need to be re-checked. However, I don't think it's necessary to have a timestamp file at all. Rather than placing a timestamp (.mdlintstamp) file in the tools directory each time a full verification is completed, I think it would be more intelligent to run touch ./tools/lint-md.js, which would update the timestamp of the actual linter doing the verification. Likewise, a successfully rebuilt linter would automatically have its timestamp updated. That way, it would be known that any files newer than the timestamp of this linter file would need to be re-verified. What do you think? |
Sorry, something went wrong.
|
I should add that each time the linter is re-built, all verifiable files would then need to be touched, which would cause their timestamps to be newer than the linter file (Markdown newer than linter == unverified). This would result in them needing to be re-verified. Here's how it would work in practice. Building the linter (run make lint-md-rollup)
Linting the docs (run make lint-md)
It's just an idea if interested in doing away with the stamp file. If not, LGTM. |
Sorry, something went wrong.
|
I think having a separate timestamp file like we do now is preferable. If someone is having weird issues with their files not being linted (been there!) and they just want to hit a reset button and have all files re-linted (been there too!), it's easy with a timestamp file: Just remove it. In the scenario you describe, we'd either have to "touch" all lintable files (yikes), artificially set the timestamp on lint-md.js to something in the (probably distant) past, or introduce an otherwise-unnecessary option like a -f flag or something that will then likely mean another Makefile task. |
Sorry, something went wrong.
|
Also, in the current setting, the Makefile itself handles most of the timestamp magic. If we move it to be the timestamp of the lint-md.js file, we lose most or all of that. |
Sorry, something went wrong.
|
Very good points. Perhaps having an independent stamp file would be preferable. |
Sorry, something went wrong.
|
Might end up having a high overhead, but did you consider creating an MD5 hash of the files instead of the timestamp? That would be cross platform compatible AFAIK |
Sorry, something went wrong.
Timestamp is already implemented. I modified the existing implementation.I did not consider replacing it with a completely new implementation. If someone wants to experiment with other implementations, that's fine, of course. I don't plan on doing that here. |
Sorry, something went wrong.
|
Another benefit to this change is that the doc files are linted even if there are failures in the "misc" files. If you look at the current lint results for #31976, you'll see that the errors in the misc files show up but then the linter bails out and it doesn't lint the doc file changes which also have lint issues. I'd prefer it always report all the problems, which will be the case with this change. |
Sorry, something went wrong.
Update Makefile so that only markdown files that have changed will be linted. Currently, if one file in doc/api has changed, all files in doc/api are linted. On Windows, the lint-md task currently lints all files regardless of whether any files has changed, and that behavior is unchanged here. A further improvement is that when tools/lint-md.js is rebuilt, the timestamp file is removed so that all files are linted again. This is because rebuilding lint-md.js can introduce new rules or modify existing rules, so re-linting everything helps make sure that accidental breakage doesn't slip by unnoticed. PR-URL: nodejs#31923 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Update Makefile so that only markdown files that have changed will be linted. Currently, if one file in doc/api has changed, all files in doc/api are linted. On Windows, the lint-md task currently lints all files regardless of whether any files has changed, and that behavior is unchanged here. A further improvement is that when tools/lint-md.js is rebuilt, the timestamp file is removed so that all files are linted again. This is because rebuilding lint-md.js can introduce new rules or modify existing rules, so re-linting everything helps make sure that accidental breakage doesn't slip by unnoticed. PR-URL: #31923 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Update Makefile so that only markdown files that have changed will be linted. Currently, if one file in doc/api has changed, all files in doc/api are linted. On Windows, the lint-md task currently lints all files regardless of whether any files has changed, and that behavior is unchanged here. A further improvement is that when tools/lint-md.js is rebuilt, the timestamp file is removed so that all files are linted again. This is because rebuilding lint-md.js can introduce new rules or modify existing rules, so re-linting everything helps make sure that accidental breakage doesn't slip by unnoticed. PR-URL: #31923 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Update Makefile so that only markdown files that have changed will be linted. Currently, if one file in doc/api has changed, all files in doc/api are linted. On Windows, the lint-md task currently lints all files regardless of whether any files has changed, and that behavior is unchanged here. A further improvement is that when tools/lint-md.js is rebuilt, the timestamp file is removed so that all files are linted again. This is because rebuilding lint-md.js can introduce new rules or modify existing rules, so re-linting everything helps make sure that accidental breakage doesn't slip by unnoticed. PR-URL: #31923 Reviewed-By: Anna Henningsen <anna@addaleax.net>
Update Makefile so that only markdown files that have changed will be linted. Currently, if one file in doc/api has changed, all files in doc/api are linted. On Windows, the lint-md task currently lints all files regardless of whether any files has changed, and that behavior is unchanged here. A further improvement is that when tools/lint-md.js is rebuilt, the timestamp file is removed so that all files are linted again. This is because rebuilding lint-md.js can introduce new rules or modify existing rules, so re-linting everything helps make sure that accidental breakage doesn't slip by unnoticed. PR-URL: #31923 Reviewed-By: Anna Henningsen <anna@addaleax.net>
| Back | FazBrowse Home | New Git URL |
Update Makefile so that only markdown files that have changed will be
linted. Currently, if one file in doc/api has changed, all files in
doc/api are linted.
On Windows, the lint-md task currently lints all files regardless of
whether any files has changed, and that behavior is unchanged here.
A further improvement is that when tools/lint-md.js is rebuilt, the
timestamp file is removed so that all files are linted again. This is
because rebuilding lint-md.js can introduce new rules or modify existing
rules, so re-linting everything helps make sure that accidental breakage
doesn't slip by unnoticed.
Checklist