These anchor tags provide compatibility with links created before the heading text was changed (anchors are automatically
generated for headings). Placing them below the headings caused the page to be scrolled to below the headings when
visited via their links.
Previously, the submission list and the Library Manager index source list were both named "repositories.txt". I found having two files with similar content, but different purposes makes the already complex system unnecessarily difficult to understand.
Unlike the index source list, the submission list is solely a list of repositories, so its current "repositories.txt"
name is appropriate. The index source list contains other information unrelated to the library repository, so the
"repositories.txt" name is not so appropriate. Since the repository is named "library-registry", the filename
"registry.txt" makes sense.
If the PR is detected as something other than a submission, review is requested from a maintainer. It might be that the
PR was intended to be a submission and the submitter is able to resolve whatever caused it to be detected as otherwise
on their own. In this case, the automated system can handle things and review is no longer needed.
Review requests to the PR author are not allowed. Previously, there would be a spurious workflow failure for every PR
from a user in the `env.MAINTAINERS` array.
When a maintainer is submitting a PR, they can request reviews as needed, so the automated review request can simply be
skipped in this case.
Previously, the submission system was configured to automatically request reviews from the `arduino/team_tooling` team.
This doesn't work, likely due to the access token provided by GitHub not having the necessary permissions. However,
requesting from individual users works fine and in the end bothering the whole Tooling Team is probably a bad idea.
The library folder name is a factor in the Arduino Lint results. Although the library.properties `name` value is used for
the Library Manager installation path, Arduino Lint rules also consider manual installation of libraries. These results
will be most accurate if the library is located in a folder named after the repository.
We are accustomed to using the `actions/checkout` GitHub Action action in workflows whenever a repository needs to be
installed into the runner. So it may be quite unexpected to see `git clone` used.
In this case, it was necessary to do so in order to clone the library repository to run Arduino Lint on it because
`actions/checkout` can only be used with GitHub repositories, whereas multiple major Git hosts are supported for
submissions to Library Manager. This might not be obvious to those working on the workflow in the future, so it's worth
leaving a comment to avoid confusion.
The "modify" request type is when the PR does removals as well as additions to the list. The valid reason for doing this
would be changing the URL of a library after the repo is renamed or the library moved to another repo.
This request type requires a manual review and merge, but the new URLs should be automatically processed to save the
reviewer from having to check it and work with the library submitter to resolve any issues that are found.
In this case, the `arduino/arduino-lint-action`'s `library-manager` input needs to be set to "update" instead of the
previously hardcoded "submit". The correct setting will be provided by the parser, so the workflow only needs to
implement the use of that setting.
Arduino Lint prints a summary report of the result of linting Arduino projects to stdout. It also offers the option of saving a JSON formatted report to a file.
In addition to rejecting the submission on an error result from Arduino Lint, the workflow also advocates for best
practices in the libraries of Library Manager by commenting a copy of the report to the PR thread if any warnings are
generated.
The machine-readable JSON format of the report file makes it easy to parse in the workflow to determine the warning
count. However, this JSON format is not terribly friendly to human readers. The `text` format report printed to stdout is
intended for that purpose. Previously, the JSON formatted report was commented to the PR thread, resulting in an
unpleasant experience for the submitter.
In the intended application of the `arduino/arduino-lint-action` GitHub Actions action, the report is printed to the log,
the interested user can access the report in the workflow run log, and any machine applications use the report file.
However, in this specialized use case, we need both a text format and a JSON format report file. Although that capability
could be added to the action, it would not likely be of use for other applications. For this reason, it makes more sense
to simply use the Arduino Lint application directly in the workflow. This really doesn't introduce any significant
complexity, since the action is only a thin wrapper.
A workflow artifact is used to transfer the PR diff file from the `diff` job to the `parse` job. Once the artifact has
been downloaded by the `parse` job, it no longer serves any purpose.
It's possible the artifact might serve as a vector for exporting secrets from the workflow. Even though I don't have any
specific reasons to believe it is possible to cause secrets to be written to the artifact and the repository doesn't
currently have any secrets beyond `GITHUB_TOKEN`, nor need for any, it's still best to remove the unnecessary artifact.
The workflow already handles all expected failures in a manner that is as automated and friendly to the submitter as
possible.
However, there is always the chance for unexpected failures caused by a bug or service outage, which are in no way the
fault of the submitter. In this event, the workflow would previously fail without any clear explanation of what had
happened. This would be likely to cause confusion to the submitter. Since the system is very automated, this failure
might also go unnoticed by the repository maintainers.
A better way to handle unexpected failures is to:
- Add a special label ("status: maintenance required").
- Request a review from the Tooling Team.
- Comment to explain to the submitter that something went wrong and we will investigate.
GitHub Actions workflow jobs default to the `if: success()` configuration. In this configuration, the job only runs when
the result of its job dependencies was success. When configuring a job to run on a failure result with `if: failure()` it
is logical to assume that the behavior would be inverted: the job would run only when the result of its dependency job
was failure. It does this, but also runs when its dependency job was canceled due to a failure of its own dependency.
This behavior of GitHub Actions resulted in the failure handling jobs running when they were not intended to. That is
avoided by specifying the exact job whose failure they were intended to handle in the conditional. It is still necessary
to use failure() in the conditional, otherwise they retain the default success() configuration and never run on a failure.
In the event a PR is detected as something other than a library submission, a review is requested from the Tooling Team
and a comment is made to the PR thread explaining the situation.
Previously, this job was named `request-review`. However, there are other circumstances under which a review will be
requested (e.g., merge conflict). So this was not a very good job name.
This job name is not referenced anywhere else in the workflow, so it currently only serves a documentation role and
changing it has no functional effect.
The `octokit/request-action` GitHub Actions action is designed to accept API request parameters as arbitrary action
inputs in the workflow. Because they have made no attempt to define all possible input keys in the action metadata,
normal and correct usage of the action cases GitHub Actions to display warnings in the workflow run log and summary. For
example:
Unexpected input(s) 'owner', 'repo', 'issue_number', 'labels', valid inputs are ['route', 'mediaType']
This has the potential to cause confusion, so I added a comment to the workflow explaining that this warning is expected
and doesn't indicate a problem. That comment somehow ended on a random occurence of `octokit/request-action` in the
workflow. It makes most sense to put it on the first usage of the action in the workflow, to make the information easy to
find.
It's certain that merge conflicts will occur as submitters take time to resolve blocking issues and other submissions are
accepted in the meantime. The PR merge will fail in this case.
Previously there was no provision for handling merge conflicts. The workflow run just failed at the merge job with no
comment from the bot.
The goal is for the automated system to work with the submitter as much as possible. Towards this goal, when the merge
fails, the bot will now comment to explain the problem and provide a link to a general tutorial for using the GitHub PR
merge conflict system.
A review is requested from the Tooling Team so that they can assist in the event the user is unable to resolve the merge
conflict, or the failure was caused by something else.
There is no good reason for a submission to consist of more than one commit.
As the submitter works with the bot to produce a compliant submission, they will sometimes end up with PRs that consist
of multiple non-atomic commits, which would pollute the repository's commit history if not squashed at the time of the
merge.
The "Manage PRs" workflow can be triggered by either a `pull_request_target` or `issue_comment` event. This means that
the `issue_number` parameter of the GitHub API request must be defined using both the `github.event.pull_request.number`
and `github.event.issue.number` properties because a different one is defined depending on which event was the trigger.
The exception is the API request for the bot comment used to provide immediate feedback when the workflow is triggered by
a comment. This API request is only ever used with an issue_comment event trigger, so the
github.event.pull_request.number property is not needed.
When there is a problem with the submission that must be resolved in the library repository, a comment mentioning
`@ArduinoBot` is used to trigger the "Manage PRs" workflow.
We are accustomed to seeing the checks status in the PR thread when workflows are running. However, with a comment
triggered workflow this does not happen. The only indication of the workflow running is on the "Actions" tab. This can
leave the submitter wondering if their comment had any effect as the workflow takes some time to run before providing
feedback.
This uncertainty can be avoided by making the bot immediately comment to acknowledge that the check is in progress.
The Library Manager indexer logs are available for each library in the index. Example:
http://downloads.arduino.cc/libraries/logs/github.com/cmaglie/FlashStorage/
This can be a valuable tool for monitoring the indexing of releases, allowing the library authors to troubleshoot without
the need for assistance from a maintainer of this repository.
Previously, as soon as a submission was compliant, the PR was merged silently. This might leave the submitter wondering
whether the submission process is complete.
There is some uncertain delay for the library release to be added to the server, the index to be generated, and the index
to propagate through the CDN, which might result in an increased support burden as the submitters comment or open issues
asking what is happening.
This is avoided by making the bot comment on the PR thread after the PR is merged, notifying the submitter that the
process was successful and that there will be some delay before the library is available from Library Manager.
Indexer logs URLs are provided for each submission, allowing the submitter to monitor the indexing process, both
immediately after the acceptance, as well as after they make future releases.
The "Manage PRs" GitHub Actions workflow generates a matrix job for each library submitted by the PR. The default job
name is generated from the job's matrix object. This contains the complete submission data, which results in a long and
somewhat cryptic job name that can make the workflow run more difficult to interpret.
The only necessary information is the description of the job's purpose ("check") and the submission URL (multiple URLs
per PR are supported). A custom job name allows for only using this information in the job name.
The official policy is that anyone is allowed to submit any library to Library Manager, regardless of whether they have
any involvement with the development and maintenance of the library.
As someone very much involved in the submission process, I have always wondered this myself. So it is very important to
clearly document this.
Some of the wording of the existing documentation implied that only the owner of the library could submit it, so this
text has been adjusted as well.
