-
Notifications
You must be signed in to change notification settings - Fork 6
Add template installation script and documentation #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or 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 failure of the "Check Workflows" CI run is caused by a newly introduced error in the GitHub Actions workflow JSON schema. That spurious validation failure will be resolved once SchemaStore/schemastore#1678 is merged (usually only a matter of a few days response time in that repo), |
silvanocerza
approved these changes
Jun 28, 2021
This is the unmodified installation script and instructions imported from Arduino Lint: - https://github.com/arduino/arduino-lint/blob/9bdca8ba87acececab868dece83044b10563bb56/etc/install.sh - https://github.com/arduino/arduino-lint/blob/9bdca8ba87acececab868dece83044b10563bb56/docs/installation.md - https://github.com/arduino/arduino-lint/blob/9bdca8ba87acececab868dece83044b10563bb56/.github/workflows/test-install.yml
This is the standardized shell script formatting style to be used in all Arduino Tooling projects.
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.
The template installation script will be made non-project specific, to be configured for each project it is applied to. However, for testing it is necessary to pick an arbitrary application to install. Although the text replacement approach taken here is crude, it has the advantage of not requiring any modifications be made to the script solely for the sake of allowing the test. Those modifications would make this important script more fragile and difficult to maintain. It's better for that fragility and maintenance burden instead be borne by the test workflow itself.
… script Use of an obvious placeholder value will make it more clear where the template installation script's configuration must be modified for the individual project it is applied to. I left the PROJECT_OWNER configuration variable unchanged because it's likely that all repositories where the Tooling Team uses this script will be under the arduino GitHub organization.
Variable names made reference to the specific project the script originated from. This would be confusing when it was used in other projects. This is a pure refactoring without any functional effect.
The installation instructions contained project-specific references from the document's origin. These are replaced with placeholder text to make it serve as a template.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
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.
This shell script provides the option to install the application with a single command.
It defaults to installing the latest release to
./bin/
, but the user also has the option of specifying a version and installation path.This is based on the installation script, instructions, and test workflow from Arduino Lint:
For this pull request, I have limited the changes to:
The functionality of the script and documentation should be unchanged.
A test workflow uses the script in various configurations to install Arduino Lint in order to verify the script is working correctly.