build: move package.json validation to make recipe #9520
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
| # Check if metadata fields need to be updated in package.json files of affected packages: | ||
| dirs=$(echo "${files_to_process}" | tr ' ' '\n' | \ | ||
| xargs dirname | \ | ||
| sed -E 's/\/(benchmark|bin|data|docs|etc|examples|include|lib|scripts|src|test)(\/[^@]*)?$//' | \ |
There was a problem hiding this comment.
Note: Replaced (\/.*)? with (\/[^@]*)? to avoid converting paths like ./lib/node_modules/@stdlib/utils/lib to ./, which is undesired (we would want ./lib/node_modules/@stdlib/utils directory in this example). By forbidding @, we can ensure that only lib folders after /@stdlib/ will be stripped.
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
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