build: add workflow and script to leave comment with make instructions #5640

Planeshifter · 2025-03-01T19:27:17Z

Description

What is the purpose of this pull request?

This pull request:

adds slash command for posting a PR-specific command comment with relevant make command.

Related Issues

Does this pull request have any related issues?

This pull request:

resolves [RFC]: Add slash command for posting a PR-specific command comment metr-issue-tracker#9.

Questions

Any questions for reviewers of this pull request?

When multiple packages are modified in one PR, the command currently does not leave any comment. Maybe that's not desirable. We could post a generic comment of the relevant commands where the expectation would then be that contributors themselves have to set the relevant environment variables. Thoughts?

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

--- 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: missing_dependencies - 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_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 ---

.github/workflows/slash_commands.yml

--- 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 ---

kgryte · 2025-03-02T02:14:24Z

the command currently does not leave any comment. Maybe that's not desirable. We could post a generic comment of the relevant commands where the expectation would then be that contributors themselves have to set the relevant environment variables. Thoughts?

Yes, we probably need to leave some sort of comment; otherwise, devs are going to wonder what's wrong and why the command didn't do anything. Devs aren't going to be aware as to why the comment never posts and they may just wait around hoping that one eventually shows up.

Planeshifter · 2025-03-02T14:25:31Z

@kgryte Now leaving the following comment when multiple packages are changed:

====

Hello! 👋

Pro-tip: This PR changes multiple packages. You can use various make rules with *_FILTERS environment variables to run tests, benchmarks, and examples for specific packages.

For each of the commands, please run them from the root stdlib repository directory (not the package folder!).

You can use pattern matching to target specific packages as follows:

# Run tests for all packages in the math namespace:
make test TESTS_FILTER=".*/@stdlib/math/.*"

# Run benchmarks for specific packages (using OR pattern):
make benchmark BENCHMARKS_FILTER=".*/@stdlib/math/base/special/sin/.*|.*/@stdlib/math/base/special/cos/.*"

Changed Packages

@stdlib/math/base/special/tan
@stdlib/math/base/special/tand
@stdlib/math/base/special/tanh

Documentation Links

--- 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 --- --- 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_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_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 ---

…s acknowledgment --- 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 --- --- 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 ---

Planeshifter · 2025-03-02T14:39:35Z

/stdlib rebase

kgryte · 2025-03-02T20:49:00Z

@Planeshifter Is there a reason you keep force pushing to branches? This ends up messing with people's (i.e., my) local copies of your PRs.

kgryte · 2025-03-02T20:49:48Z

Also, s/*_FILTERS/*_FILTER. The filter environment variables don't end with an S.

kgryte · 2025-03-02T20:52:21Z

As an additional point, your OR example can be simplified:

make benchmark BENCHMARKS_FILTER=".*/@stdlib/math/base/special/(sin|cos)/.*"

Planeshifter · 2025-03-02T20:54:17Z

@kgryte I accidentally pushed a commit to this PR that was supposed to go to develop and is unrelated to this PR. I meant to fix this via a force push, but somehow the commands I would usually do to accomplish this (i.e., git reset HEAD~1 --hard followed by git push origin philipp/package-make-commands-comment --force) didn't work. There is definitely something strange going on with our git commit hashes; potentially due to the pre-commit hooks? We should look into this; similar to @rreusser, I have recently encountered some strange behavior with pushes and pulls.

kgryte · 2025-03-02T20:55:44Z

Yeah, we may need to abandon appending commit meta data to commits.

--- 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 --- --- 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_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 ---

Planeshifter · 2025-03-02T21:08:46Z

@kgryte Only the pre-push report does invoke git commit --amend. My hunch is that's the root of the problem because when we amend a commit, we are essentially creating a new commit with a different hash. The pre-push results seem less important than the pre-commit ones, so maybe we could consider disabling these to start with?

kgryte · 2025-03-02T21:16:00Z

Yeah that is fine. Both are proxy for whether a dev has setup their local env. Feel free to change. I am still down and out.

.github/workflows/scripts/package_commands_comment

Signed-off-by: Athan <[email protected]>

.github/workflows/scripts/package_commands_comment

Signed-off-by: Athan <[email protected]>

.github/workflows/scripts/package_commands_comment

Signed-off-by: Athan <[email protected]>

kgryte

LGTM. Although, in general, I would really like to get away from embedding documentation directly into Bash scripts like this.

Ideally, we would create separate Markdown template files which can be linted and maintained just like other Markdown files. Having all these escapes and unorthodox indentation practices thwarts find-and-replace and general maintenance practices.

Planeshifter requested a review from kgryte March 1, 2025 19:27

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

Planeshifter force-pushed the philipp/package-make-commands-comment branch from 37235de to 3774388 Compare March 1, 2025 19:28

Planeshifter mentioned this pull request Mar 1, 2025

[RFC]: Add slash command for posting a PR-specific command comment stdlib-js/metr-issue-tracker#9

Closed

kgryte reviewed Mar 1, 2025

View reviewed changes

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

Planeshifter requested a review from kgryte March 1, 2025 23:58

kgryte previously approved these changes Mar 2, 2025

View reviewed changes

kgryte added Tools Issue or pull request related to project tooling. CI Issue or pull request specific to continuous integration environments. Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Mar 2, 2025

Planeshifter dismissed kgryte’s stale review via f29a736 March 2, 2025 14:24

Planeshifter force-pushed the philipp/package-make-commands-comment branch from f29a736 to 8b4e5ca Compare March 2, 2025 14:27

stdlib-bot added bot: In Progress Pull request is currently awaiting automation. and removed bot: In Progress Pull request is currently awaiting automation. labels Mar 2, 2025

Planeshifter added Needs Review A pull request which needs code review. and removed Needs Changes Pull request which needs changes before being merged. labels Apr 5, 2025

Planeshifter requested a review from kgryte April 5, 2025 18:40