Skip to content

tools: Warn when ARMC5 is no longer supported #13008

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
merged 2 commits into from
May 27, 2020

Conversation

Patater
Copy link
Contributor

@Patater Patater commented May 22, 2020

Summary of changes

For backwards compatibility reasons, as the Mbed tools must be able to build any online-compiler-supported version of Mbed, revert back to emitting a warning when ARMC5 is used. This will enable ARMC5 to continue working in the online compiler for Mbed OS 5 and other previous versions. ARMC5 remains unsupported in Mbed 6.

Impact of changes

None

Migration actions required

None

Documentation

No additional documentation required.


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


Patater added 2 commits May 22, 2020 15:39
…olchain."

For backwards compatibility reasons, as the Mbed tools must be able to
build any online-compiler-supported version of Mbed, revert back to
emitting a warning when ARMC5 is used. This will enable ARMC5 to
continue working in the online compiler for Mbed OS 5 and other previous
versions. ARMC5 remains unsupported in Mbed 6.

This reverts commit 03d57b7.
Update the warning when ARMC5 is used to specify the conditions under
which ARMC5 is no longer supported.
@Patater Patater changed the title Armc5 usage warning compat tools: Warn when ARMC5 is no longer supported May 22, 2020
@ciarmcom ciarmcom requested review from a team May 22, 2020 15:00
@ciarmcom
Copy link
Member

@Patater, thank you for your changes.
@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

If this goes to master, and we apply this to the online compiler - will it work for 5.15.x as well ? we dont need to propagate this patch to 5.15 branch as well?

Otherwise, this make sense

Copy link
Contributor

@mark-edgeworth mark-edgeworth left a comment

Choose a reason for hiding this comment

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

This looks ok as a first step, although all this does is to change the error to a warning of course. The proof here will be whether the code actually still generates a working build for ARMC5...

@mergify mergify bot added needs: CI and removed needs: review labels May 26, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented May 26, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented May 26, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 1
Build artifacts

Copy link
Collaborator

@andypowers andypowers left a comment

Choose a reason for hiding this comment

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

Approved.

@Patater
Copy link
Contributor Author

Patater commented May 26, 2020

If this goes to master, and we apply this to the online compiler - will it work for 5.15.x as well ? we dont need to propagate this patch to 5.15 branch as well?

No, this won't be needed on the 5.15 branch. The online compiler uses the tools from the latest version of Mbed.

Sorry this doesn't make sense.

@0xc0170 0xc0170 merged commit c7759fe into ARMmbed:master May 27, 2020
@mergify mergify bot removed the ready for merge label May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants