-
Notifications
You must be signed in to change notification settings - Fork 80
CI: Use mbed_portenta core release instead of ArduinoCore-mbed HEAD #333
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
Memory usage change @ 82086a5
Click for full report table
Click for full report CSV
|
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 change is valid, but the description of the change is not really correct.
The previous approach was intentionally used to test the library against the version of the "Arduino Mbed OS Boards" platform from the tip of its repository's default branch.
The change made here causes the library to be tested against the release version of the "Arduino Mbed OS Boards" platform. If that is what is wanted, then great, that is a perfectly reasonable approach. I only want to make sure the true result of the proposal is understood.
@@ -128,18 +126,16 @@ jobs: | |||
- examples/utility/Provisioning | |||
# Portenta | |||
- board: | |||
type: mbed | |||
type: mbed_portenta | |||
platforms: | | |||
# Install Arduino mbed-Enabled Boards via Boards Manager for the toolchain |
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.
# Install Arduino mbed-Enabled Boards via Boards Manager for the toolchain |
It is no longer being installed for the toolchain alone. The platform is now actually being used instead of being replaced by the one sourced from the Git repository.
@per1234 Thanks for the check. The PR title and the commit description was definetly wrong. I've changed the title and then i will also change the commit descriprion if we decide to merge this. What is the reason to test against ArduinoCore-mbed HEAD? This is done only for Portenta, all the other boards are using a released core. In the WebEditor and in IoTCloud we are also using a released core. I think that using the last released core will cover more uses cases than using ArduinoCore-mbed HEAD. |
I did a quick investigation via blame and found this:
So it appears this was a provisional measure that ended up being left in place two years longer than necessary. |
👍 Great so i think we can go on with this PR. I will force push to fix the commit description and remove the comment about the toolchain. Bonus point: we have already removed the M4 ci build because ArduinoIoTCloud can't run on it. #310 |
Either way you want to handle it is fine with me. |
82086a5
to
d404e8d
Compare
Memory usage change @ d404e8d
Click for full report table
Click for full report CSV
|
This will also fix this Ci compile examples error :