Skip to content
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

Updated arduino-cli to 0.35.x branch with a lot of libraries #761

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Oct 1, 2024

This brings the Arduino CLI integration up to 0.35.x branch (the latest before the big 1.0 break).
I've taken the chance to upgrade golang to 0.22.5 (we are still not supporting 0.23.0 in the Disttask.yml file).

Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.97%. Comparing base (cf59441) to head (0c7f6cf).
Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #761      +/-   ##
==========================================
- Coverage   90.05%   89.97%   -0.09%     
==========================================
  Files          44       44              
  Lines        6800     6772      -28     
==========================================
- Hits         6124     6093      -31     
- Misses        553      555       +2     
- Partials      123      124       +1     
Flag Coverage Δ
unit 89.97% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmaglie cmaglie self-assigned this Oct 1, 2024
@cmaglie cmaglie added type: enhancement Proposed improvement dependencies labels Oct 1, 2024
@cmaglie cmaglie force-pushed the update-extract-lib branch 4 times, most recently from de93d49 to 48471b7 Compare October 2, 2024 14:07
@per1234 per1234 added topic: infrastructure Related to project infrastructure and removed dependencies labels Oct 8, 2024
@per1234
Copy link
Contributor

per1234 commented Oct 16, 2024

I apologize for introducing merge conflicts for this PR in the workflows @cmaglie. I have a couple more changes to make in the workflows to get them fully up to date and I'll take care of rebasing the PR once those are done. I'm not touching any Go code or dependencies or integration tests.

@cmaglie
Copy link
Member Author

cmaglie commented Oct 16, 2024

I apologize for introducing merge conflicts for this PR in the workflows @cmaglie.

No worries, I've already rebased it, the changes were trivial.

@cmaglie
Copy link
Member Author

cmaglie commented Oct 16, 2024

@per1234 I removed the checks for sketch.json from the ruleset since it's now deprecated in favor of sketch.yaml, that's the reason for the small reduction in the test coverage.

I want to add checks for the sketch.yaml, but IMHO this should be done after we merge the support for the CLI 1.0+.

Copy link

@alessio-perugini alessio-perugini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

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.

This needs to be updated:

GO_VERSION: 1.17

The reason I did this is because I found that, without it, the project had a lot of unused transitive dependencies (for example, when I updated a direct module dependency and did a go mod tidy, a former dependency of that direct dependency that was no longer used by the new version would be left behind in the go.mod+go.sum, and thus in the license metadata, even though it was no longer used by anything). Once I added the -compat=1.17 flag, go tidy behaved exactly as I expected. Apparently the unexpected behavior was due to Go needing to do a suboptimal tidy in order to provide compatibility with certain other versions of Go, but we don't support the use of the project with versions of Go other than the one specified by the go directive so that isn't of interest.

I don't know whether the -compat flag is needed for proper tidy behavior with 1.22 though.


This needs to be updated:

- [Go](https://golang.org/doc/install) version 1.17 or later

Or we can merge #801 now and render it obsolete:

- [Go](https://golang.org/doc/install
- The **Go** version in use is defined in the `go` directive of
[`go.mod`](https://github.com/arduino/arduino-lint/blob/main/go.mod).

go.mod Outdated Show resolved Hide resolved
@cmaglie
Copy link
Member Author

cmaglie commented Oct 18, 2024

Or we can merge #801 now and render it obsolete:

- [Go](https://golang.org/doc/install
- The **Go** version in use is defined in the `go` directive of
[`go.mod`](https://github.com/arduino/arduino-lint/blob/main/go.mod).

With the latest go versions, the compiler should automatically set compatibility mode with the toolchain version specified in the go.mod file, so it should be enough to download a version of the toolchain that is >= of the required version. This differs from older releases, where you had to download the exact version of go used for the build. At least, this is my experience. Maybe we can try to build this project with go 1.23 to confirm it.

@cmaglie cmaglie requested a review from per1234 October 18, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants