-
-
Notifications
You must be signed in to change notification settings - Fork 12
Add release installation support #123
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
Conversation
CHECKLATESTVERSION_REGEX="[0-9][A-Za-z0-9\.-]*" | ||
CHECKLATESTVERSION_LATEST_URL="https://github.com/${PROJECT_OWNER}/${PROJECT_NAME}/releases/latest" | ||
if [ "$DOWNLOAD_TOOL" = "curl" ]; then | ||
CHECKLATESTVERSION_TAG=$(curl -SsL $CHECKLATESTVERSION_LATEST_URL | grep -o "<title>Release $CHECKLATESTVERSION_REGEX · ${PROJECT_OWNER}/${PROJECT_NAME}" | grep -o "$CHECKLATESTVERSION_REGEX") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rsora do you know why this approach of scraping the GitHub release page was taken in the Arduino CLI repo instead of using the "latest" link from downloads.arduino.cc (e.g., https://downloads.arduino.cc/arduino-cli/arduino-cli_latest_Linux_64bit.tar.gz
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script was borrowed from the Glide repo (https://glide.sh/), we probably used the same approach as the script (that is changed thought from the time we picked it up).
Anyway, I suggest maintaining this approach for the release, as there is no "latest" service deployed for the arduino-lint currently.
You can see a run of the installation script test CI workflow (#124) on this script, after some modifications to work around the missing and private resources that would cause the unmodified script to fail before the release, here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answered your question in the comments, LGTM
CHECKLATESTVERSION_REGEX="[0-9][A-Za-z0-9\.-]*" | ||
CHECKLATESTVERSION_LATEST_URL="https://github.com/${PROJECT_OWNER}/${PROJECT_NAME}/releases/latest" | ||
if [ "$DOWNLOAD_TOOL" = "curl" ]; then | ||
CHECKLATESTVERSION_TAG=$(curl -SsL $CHECKLATESTVERSION_LATEST_URL | grep -o "<title>Release $CHECKLATESTVERSION_REGEX · ${PROJECT_OWNER}/${PROJECT_NAME}" | grep -o "$CHECKLATESTVERSION_REGEX") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script was borrowed from the Glide repo (https://glide.sh/), we probably used the same approach as the script (that is changed thought from the time we picked it up).
Anyway, I suggest maintaining this approach for the release, as there is no "latest" service deployed for the arduino-lint currently.
It may be useful to check the ongoing development work on the source script. Previously, it was difficult to find that script because it's hosted in a different repository from the one the licensing comment provides.
Using the default configuration of shfmt.
Unfortunately, `local` is Bash-specific. The script's shebang (#!/bin/sh) indicates POSIX compliance was intended, so the use of `local` is not appropriate. I used the function name as a prefix "namespace" for the previously local variables in the attempt to achieve somewhat of the same effect in a POSIX-compliant manner.
ShellCheck is a static analysis tool for shell scripts.
I would prefer to avoid cluttering up the root of the repository too much. The reason is that when people visit the repository on GitHub, they must scroll down past the root directory listing before getting to the readme. Some files and folders must be in the root, but there are some others (e.g., gon.config.hcl) that can be moved to a subfolder, so this move can be part of a larger campaign.
arduino-lint uses the --version flag.
cf6c9d1
to
ca03ece
Compare
Note: