-
-
Notifications
You must be signed in to change notification settings - Fork 12
Rework dist process and add minimal documentation #137
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
26e6362
to
a85ab56
Compare
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.
I added the TIMESTAMP_SHORT back in the checksum filename since that should be always present even in tagged releases. This is the way it is in the CLI right now at least.
Why is it useful to have a timestamp in addition to the tag? The reason I'm questioning this is because I had the idea that people might want to be able to download the checksums for the releases just as they can currently do with the nightly. But having the timestamp in the filename will make that more difficult than simply being able to insert the version they want into the URL.
The CLI checksum file always contains the timestamp so I did the same to keep it compatible, same for nightlies. |
For the nightlies, it definitely makes sense because the timestamp is what we use as the unique identifier of nightlies. But for the releases we already have a unique identifier: the version, so I don't see that the timestamp adds any benefits, while also having a disadvantage. I think it's good to maintain consistency between arduino-lint and Arduino CLI's infrastructure where possible, but I also see this as an opportunity to have a fresh start and make improvements on Arduino CLI's infrastructure, which can then be ported back to Arduino CLI. Arduino CLI's checksum system is already broken (see below), so it needs to be be changed anyway. That could be done at the same time as this new build system you designed is put in place.
See ATL-794 |
Aha! Didn't know it was broken for the CLI, am all in to change it then. |
If a nightly is being built the version string will be "nightly-<timestamp>". Else if a tag is set in the currently checked out commit that is used. In all other cases the "snapshot" string is used, this last case handles a local build started manually by a developer.
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.
I ran the nightly and release workflows in my fork and everything worked perfectly. Great work Silvano!
I reworked a bit the
DistTasks.yml
to make it project agnostic as much as possible.The
VERSION
var can now be eithernightly
if theNIGHTLY
env var is set totrue
, a Git tag ifNIGHTLY
is not set and a tag is found in the current commit, elsesnapshot
is used. Ideally the last case handles a build done locally when checked out at an untagged commit. This change is also reflected in thenightly.yml
workflow.I added the
TIMESTAMP_SHORT
back in the checksum filename since that should be always present even in tagged releases. This is the way it is in theCLI
right now at least.