Skip to content

Provide actual string to findPlatform for Optional<Version> #8069

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 15, 2018
Merged

Provide actual string to findPlatform for Optional<Version> #8069

merged 2 commits into from
Oct 15, 2018

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Oct 8, 2018

Fix #8034

@cmaglie a better way to solve this could be passing a Version to findPlatform but this would result in an API change. Should we go that way?

@cmaglie cmaglie mentioned this pull request Oct 8, 2018
Closed
@cmaglie
Invalid use of toString in Optional<Version>
6a1c1f4 
It should be used on the actual Version.

Fix arduino#8034
@ArduinoBot
Copy link
Contributor

✅ Build completed.

Please test this code using one of the following:

⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8069-BUILD-790-linux32.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8069-BUILD-790-linux64.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8069-BUILD-790-windows.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8069-BUILD-790-macosx.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8069-BUILD-790-linuxarm.tar.xz

ℹ️ The linuxarm build is still experimental and may not be always available.

@arduino arduino deleted a comment from ArduinoBot Oct 8, 2018
@cmaglie
Copy link
Member

cmaglie commented Oct 8, 2018

@cmaglie a better way to solve this could be passing a Version to findPlatform but this would result in an API change. Should we go that way?

No, your fix is good, I've just changed it a bit. Moreover this shows that the compiler doesn't warn you if you accidentally use Optional.toString().

@cmaglie cmaglie added this to the Release 1.8.8 milestone Oct 8, 2018
@cmaglie cmaglie merged commit 6a1c1f4 into arduino:master Oct 15, 2018
@cmaglie cmaglie deleted the version_optional_regression_2 branch October 15, 2018 13:28
@gvarisco gvarisco mentioned this pull request Dec 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 1.8.10
Development

Successfully merging this pull request may close these issues.
3 participants
@facchinm @ArduinoBot @cmaglie