build: move package.json validation to make recipe#9520
build: move package.json validation to make recipe#9520Planeshifter wants to merge 4 commits intodevelopfrom
Conversation
|
/stdlib update-copyright-years |
stdlib-bot
added
bot: In Progress
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Planeshifter
force-pushed
the
philipp/move-pkg-json-validation-to-make
branch
from
c3c09bb to
f02bd2a
Compare
|
/stdlib update-copyright-years |
stdlib-bot
added
the
bot: In Progress
stdlib-bot
removed
the
bot: In Progress
lib/node_modules/@stdlib/_tools/package-json/scripts/validate_package_json_files
Show resolved
Hide resolved
kgryte
added
Tools
| # @example | ||
| # make validate-pkg-json FILES='/foo/lib/index.js /bar/package.json' | ||
| #/ | ||
| validate-pkg-json: $(NODE_MODULES) |
There was a problem hiding this comment.
This recipe is somewhat oddly named for a few reasons.
- I am not sure we have a
validate-*command anywhere else among themakerecipes. - The target doesn't follow the convention of appending
-fileswhen a recipe is intended to work with a list of files (e.g.,lint-javascript-files).
Instead, I would name this target lint-pkg-json-metadata-files.
A few other comments:
- In contrast to other linters (e.g., ESLint), this doesn't support the
FAST_FAILenvironment variable. To do so, we would do something similar to our ESLint recipes where we'd individually iterate over each file. - I think we should have a corresponding
lint-pkg-json-metadatarecipe which supports globs. Granted,lint-package-jsonabove doesn't work like that atm. This can be punted to a future PR. - I mention (2) because it is odd to me that we need two separate lint commands to fully lint a single
package.jsonfile. Because one recipe works onFILESand the other doesn't, we cannot readily create a singlelint-pkg-jsoncommand which delegates to two more specialized recipes.
Longer term, I think we would be best served by the following targets:
lint-pkg-json-schemalint-pkg-json-schema-fileslint-pkg-json-metadatalint-pkg-json-metadata-fileslint-pkg-jsonlint-pkg-json-files
where lint-pkg-json then has lint-pkg-json-schema and lint-pkg-json-metadata as prerequisites.
For this PR, I suggest going ahead and at least creating a beachhead by renaming the target and updating the various other downstream consumers in this PR accordingly. I'd also suggest refactoring the recipe to support FAST_FAIL.
There was a problem hiding this comment.
While its true that this is named somewhat differently and lacking the -files suffix that other targets have, my rationale was that it is indeed quite different, because we are not actually linting a list of files. Instead, we resolve all package.json files associated with a list of files and then check whether they need to be updated. Your comment doesn't wrestle with this crucial distinction at all, I think?
FAST_FAIL also doesn't make sense given this context
There was a problem hiding this comment.
Ah, right. This is package-level linting (e.g., if a C implementation is added to a package, then we need to confirm that the package.json is updated accordingly).
In which case, this may be better placed in make/lib/lint/pkgs. Locally, I have a WIP recipe for doing whole package linting with the target make lint-pkgs. Let me go ahead and commit that recipe.
Then, I suggest we rename to lint-pkgs-metadata-files and move the recipe to lint/pkgs. That work for you?
And yes, you're right that FAST_FAIL doesn't map well here.
There was a problem hiding this comment.
Yeah, that works. Will update PR accordingly.
kgryte
added
the
Needs Changes
|
/stdlib merge |
stdlib-bot
added
the
bot: In Progress
…-json-validation-to-make
stdlib-bot
removed
the
bot: In Progress
|
/stdlib merge |
stdlib-bot
added
the
bot: In Progress
…-json-validation-to-make
stdlib-bot
removed
the
bot: In Progress
3 participants
Resolves stdlib-js/metr-issue-tracker#62.
Description
This pull request:
package.jsonmetadata validation from thepackage.jsonlintingpackage.jsonfiles that might need to have their metadata updated for a list of filespackage.jsonmetadata validation to pre-commit hookRelated Issues
No.
Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
@stdlib-js/reviewers