Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
silvanocerza merged 2 commits into main from scerza/fix-dist-task
Jan 7, 2021

Conversation

Copy link
Contributor

@silvanocerza silvanocerza commented Jan 4, 2021

I reworked a bit the DistTasks.yml to make it project agnostic as much as possible.

The VERSION var can now be either nightly if the NIGHTLY env var is set to true, a Git tag if NIGHTLY is not set and a tag is found in the current commit, else snapshot is used. Ideally the last case handles a build done locally when checked out at an untagged commit. This change is also reflected in the nightly.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 the CLI right now at least.

Copy link
Contributor

@per1234 per1234 left a 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.

Copy link
Contributor Author

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.
At least according to the documentation, I just tried to download it but I can't seem to find the file.
I'd expect https://downloads.arduino.cc/arduino-cli/nightly/nightly-20210101-checksums.txt to work but it doesn't.

Copy link
Contributor

per1234 commented Jan 5, 2021

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.

I'd expect https://downloads.arduino.cc/arduino-cli/nightly/nightly-20210101-checksums.txt to work but it doesn't.

See ATL-794

Copy link
Contributor Author

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.

I'd expect https://downloads.arduino.cc/arduino-cli/nightly/nightly-20210101-checksums.txt to work but it doesn't.

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.
Copy link
Contributor

@per1234 per1234 left a 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!

silvanocerza reacted with heart emoji
@silvanocerza silvanocerza merged commit ff95327 into main Jan 7, 2021
@silvanocerza silvanocerza deleted the scerza/fix-dist-task branch January 7, 2021 09:27
@per1234 per1234 added topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement labels Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 per1234 approved these changes

Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /