build: ensure only errors are displayed in issue via log file #5751

Planeshifter · 2025-03-03T02:44:44Z

Description

What is the purpose of this pull request?

This pull request:

adds support for an ESLINT_ERROR_LOG variable to store errors when running eslint-files make rule.

Using the JSON formatter as suggested in the issue and discussed doesn't help much since we process each file individually and would then still need to parse out the results for them. Instead, I ended up settling on re-running the ESLint command with --quiet to ensure only errors get reported and appending the results to a log file whenever linting of a file fails.

Related Issues

Does this pull request have any related issues?

This pull request:

resolves [RFC]: Improve JS lint issue content to include only failing cases metr-issue-tracker#45.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

Read, understood, and followed the contributing guidelines.

@stdlib-js/reviewers

kgryte · 2025-03-04T00:21:32Z

Using the JSON formatter as suggested in the issue and discussed doesn't help much since we process each file individually and would then still need to parse out the results for them

Why wasn't the Unix formatter suitable?

Planeshifter · 2025-03-04T02:58:06Z

@kgryte

If we used the Unix formatter, we would get output like

/var/lib/jenkins/workspace/eslint Release/eslint/fullOfProblems.js:1:10: 'addOne' is defined but never used. [Error/no-unused-vars]

We could implement this and then use grep to filter lines containing [Error. However, I find this output less appealing than the default formatting. Second, we would need to maintain the previous tee approach in order to print to both stdout and a file then. Finally, we would need to either use a deprecated ESLint formatter (no longer available in v9) or add that external dependency. In comparison, I find the make rule variable for storing errors simple and clear.

tools/make/lib/lint/javascript/eslint.mk

.github/workflows/lint_random_files.yml

kgryte · 2025-04-16T00:50:12Z

.github/workflows/lint_random_files.yml

            if [ -f "lib/node_modules/${pkg}/binding.gyp" ]; then
                NODE_ADDONS_PATTERN="${pkg}" make install-node-addons
            fi
          done


I suggest moving this logic to a separate Bash script. This will make refactoring and linting easier.

kgryte · 2025-05-31T22:51:21Z

@Planeshifter Would it be possible to move this PR along? Would help to minimize some of the PR noise we are experiencing for the lint issues.

--- 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: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed --- --- type: pre_push_report description: Results of running various checks prior to pushing changes. report: - task: run_javascript_examples status: na - task: run_c_examples status: na - task: run_cpp_examples status: na - task: run_javascript_readme_examples status: na - task: run_c_benchmarks status: na - task: run_cpp_benchmarks status: na - task: run_fortran_benchmarks status: na - task: run_javascript_benchmarks status: na - task: run_julia_benchmarks status: na - task: run_python_benchmarks status: na - task: run_r_benchmarks status: na - task: run_javascript_tests status: na ---

--- 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: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

--- 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: passed - task: lint_typescript_declarations status: na - task: lint_typescript_tests status: na - task: lint_license_headers status: passed ---

kgryte · 2025-09-01T21:47:31Z

.github/workflows/scripts/lint_javascript_files_error_log

+# MAIN #
+
+# Initialize error file:
+: > "${err_file}"


This is valid and a trick, but not typical. One would conventionally use touch here.

kgryte · 2025-09-01T21:50:11Z

.github/workflows/scripts/lint_javascript_files_error_log

+files=$(echo "${files_to_lint}" | tr ' ' '\n' | grep '\.js$' | grep -v -e '/examples' -e '/test' -e '/benchmark' -e '^dist/' | tr '\n' ' ' | sed 's/ $//')
+if [ -n "${files}" ]; then
+	echo "Linting JavaScript source files..."
+	run_make_lint "${files}" "" ""


This parameterization of run_make_lint is a bit odd and not idiomatic. This isn't the end of the world, but the implementation could be cleaner.

kgryte · 2025-09-01T21:50:43Z

.github/workflows/scripts/lint_javascript_files_error_log

+files=$(echo "${files_to_lint}" | tr ' ' '\n' | grep '/examples/.*\.js$' | tr '\n' ' ' | sed 's/ $//')
+if [ -n "${files}" ]; then
+	echo "Linting JavaScript example files..."
+	run_make_lint "${files}" "" "ESLINT_CONF=\"${eslint_examples_conf}\""


E.g., escaping environment variable strings, etc, is odd.

PR-URL: #5751 Closes: stdlib-js/metr-issue-tracker#45 Reviewed-by: Athan Reines <kgryte@gmail.com>

stdlib-bot added the Needs Review A pull request which needs code review. label Mar 3, 2025

Planeshifter mentioned this pull request Mar 3, 2025

[RFC]: Improve JS lint issue content to include only failing cases stdlib-js/metr-issue-tracker#45

Closed

stdlib-js deleted a comment from stdlib-bot Mar 3, 2025

Planeshifter mentioned this pull request Mar 24, 2025

Fix JavaScript lint errors #6334

Closed

kgryte self-requested a review March 24, 2025 07:25

kgryte reviewed Apr 16, 2025

View reviewed changes

tools/make/lib/lint/javascript/eslint.mk Outdated Show resolved Hide resolved

kgryte reviewed Apr 16, 2025

View reviewed changes

tools/make/lib/lint/javascript/eslint.mk Outdated Show resolved Hide resolved

kgryte reviewed Apr 16, 2025

View reviewed changes

.github/workflows/lint_random_files.yml Outdated Show resolved Hide resolved

kgryte reviewed Apr 16, 2025

View reviewed changes

Planeshifter marked this pull request as draft April 25, 2025 23:49

Planeshifter added 4 commits September 1, 2025 12:16

Planeshifter force-pushed the philipp/js-lint-issues-display-only-errors branch from ed6a9d1 to 6213840 Compare September 1, 2025 17:16

Planeshifter marked this pull request as ready for review September 1, 2025 17:17

stdlib-bot added the Needs Review A pull request which needs code review. label Sep 1, 2025

kgryte removed the Needs Changes Pull request which needs changes before being merged. label Sep 1, 2025

kgryte reviewed Sep 1, 2025

View reviewed changes

kgryte approved these changes Sep 1, 2025

View reviewed changes

kgryte removed the Needs Review A pull request which needs code review. label Sep 1, 2025

kgryte merged commit c9c5917 into develop Sep 1, 2025
30 checks passed

kgryte deleted the philipp/js-lint-issues-display-only-errors branch September 1, 2025 21:52

Planeshifter added a commit that referenced this pull request Sep 8, 2025

build: ensure only errors are displayed in issue via log file

a2dd7d0

PR-URL: #5751 Closes: stdlib-js/metr-issue-tracker#45 Reviewed-by: Athan Reines <kgryte@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

build: ensure only errors are displayed in issue via log file #5751

build: ensure only errors are displayed in issue via log file #5751

Uh oh!

Planeshifter commented Mar 3, 2025

Uh oh!

kgryte commented Mar 4, 2025

Uh oh!

Planeshifter commented Mar 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kgryte Apr 16, 2025

Uh oh!

kgryte commented May 31, 2025

Uh oh!

kgryte Sep 1, 2025

Uh oh!

kgryte Sep 1, 2025

Uh oh!

kgryte Sep 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

build: ensure only errors are displayed in issue via log file #5751

build: ensure only errors are displayed in issue via log file #5751

Uh oh!

Conversation

Planeshifter commented Mar 3, 2025

Description

Related Issues

Questions

Other

Checklist

Uh oh!

kgryte commented Mar 4, 2025

Uh oh!

Planeshifter commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kgryte Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

kgryte commented May 31, 2025

Uh oh!

kgryte Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

kgryte Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

kgryte Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Planeshifter commented Mar 4, 2025 •

edited

Loading