Sync installation assets from template #217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The installation script checks the path of the effective executable after installing the application. If it is different from the installation path, it warns the user. Previously, the external tool `which` was used for this purpose. This is a violation of ShellCheck rule C2230, which results in an error with the version of ShellCheck installed in the GitHub Actions runner (0.7.0). The installation script is intended to use POSIX standard tools where possible in order to be as portable as possible. The recommended replacement `command -v` works equally as well for this particular application, so there is no reason not to make the change recommended by ShellCheck. Reference: https://github.com/koalaman/shellcheck/wiki/SC2230
These are intended to facilitate syncing fixes and improvements to the templates in either direction.
This content has multiple issues: - Contains project-specific references - Outdated (doesn't mention BINDIR) Since it needs to be modified one way or the other and we already have documentation of the script in installation.md, I think it's best to remove this altogether to avoid the need to maintain two copies of the documentation, and the likelihood that it will not be maintained.
Variable names made reference to the specific project the script originated from. This is not appropriate now that the installation script is intended to be used for any project. This is a pure refactoring without any functional effect.
From https://github.com/koalaman/shellcheck/wiki/SC2268 > Some older shells would get confused if the first argument started with a dash, or consisted of ! or (. As a > workaround, people would prefix variables and values to be compared with x to ensure the left-hand side always started > with an alphanumeric character. > > POSIX ensures this is not necessary, and all modern shells now follow suite. This obsolete practice is used by the template installation script in a comparison of the release's Git tag name. Since it would be extremely unusual to start a tag name with any of these characters (or impossible in the case of `-`, since it is prohibited by Git), it's unlikely this would be necessary even in the days when this bug was prevalent in shells. In addition, this bug was fixed quite some years ago, making it unlikely that it even exists on any user's system. So this practice makes the script code more complex without providing any real benefit. Since it is a violation of ShellCheck rule SC2268 and causing a CI failure, it is best to just remove it.
A "right single quotation mark" (U+2019) somehow ended up being used as an apostrophe in the installation instructions.
per1234
added
type: enhancement
silvanocerza
approved these changes
Aug 2, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We have assembled a collection of reusable project assets:
https://github.com/arduino/tooling-project-assets
These assets will be used in the repositories of all Arduino tooling projects.
Some minor improvements and standardizations have been made there to the installation script and documentation, and these are introduced via this pull request.