Target specific scripts in "Check Scripts" template #421
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Previously, the approach taken in the "Check Scripts" template was to recursively search the entire repository for shell scripts. There were several problems with that system:
The
-regextype
flag used in thefind
commands of theshell:check
andshell:check-mode
tasks is not supported by the BSD/macOS version offind
, meaning the tasks would fail if a contributor using a macOS machine tried to run them:The
shell:check
andshell:check-mode
tasks only provided coverage for files with.sh
and.bash
file extensions, whereas the practice of omitting a file extension on shell scripts is unfortunately quite common.There was no obvious way to exclude paths of externally maintained files from coverage by the
shell:format
task.Alternative Solutions
The first of the problems listed above could be overcome by configuring the tasks to adjust the
find
commands to use different flags depending on the operating system of the machine (-regextype posix-extended
when running in a GNU shell and-E
when running in a BSD shell).The second could be overcome by using
shfmt --files
(which searches recursively for shell scripts by checking for the presence of a shebang inside files and outputs a list of the paths) as a replacement forfind
.The third could be overcome by documenting the use of shfmt's poorly advertised feature of ignoring the paths assigned an
ignore
attribute in the.editorconfig
file.Chosen Solution
After careful consideration, the decision was made to abandon the previous approach of attempting to automatically discover script files and instead use the strategy of configuring the template with the specific paths of the scripts to be checked for each project it is installed into.
Although such an approach would not be appropriate in the case of a check on files that tend to be present in greater abundance and regularly added and moved in a project, this is not the case for shell scripts in Arduino projects. A project is more likely to contain a few scripts at most and their paths tend to be reasonably stable.
Code Duplication in Workflow
In order to make it easier to interpret results, navigate logs, and reduce duration of workflow runs, a separate workflow job is used for each of the distinct checks. Unfortunately it is necessary for the project maintainer to configure the script paths redundantly in the matrix of each of the three jobs.
Intuitively we would expect this could be avoided by defining the paths at workflow scope via the
env
key. However, this is not feasible due to theenv
context not being available for use in thejobs.<job name>.strategy.matrix
key.It is technically possible to accomplish this by adding a job that converts the data from the
env
context into a job output (theneeds
context is available for use in thejobs.<job name>.strategy.matrix
key). However the significant increase in complexity this would bring to the workflow outweighs the benefit of avoiding duplication.