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

Remove unused tools modules and document the used ones. #10254

Merged
merged 14 commits into from Apr 9, 2019

Conversation

theotherjimmy
Copy link
Contributor

Description

These directories have contained exclusively dead code for as long as
I can remember. Now is as good of a time as any to remove them.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@theotherjimmy theotherjimmy changed the title Remove tools/compliance and tools/dev Remove unused tools modules Mar 28, 2019
@ciarmcom ciarmcom requested a review from a team March 28, 2019 16:00
@ciarmcom
Copy link
Member

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

@ciarmcom ciarmcom requested a review from a team March 28, 2019 16:00
@theotherjimmy theotherjimmy changed the title Remove unused tools modules Remove unused tools modules and document the used ones. Mar 28, 2019
@bridadan
Copy link
Contributor

@theotherjimmy
Copy link
Contributor Author

@bridadan They're not imported?

@bridadan
Copy link
Contributor

@theotherjimmy Nope, just checked

@bridadan
Copy link
Contributor

Most of test_webapi.py is commented out 😆

tools/README.md Outdated
@@ -45,6 +45,5 @@ Quick navigation:
| `test_webapi.py` | part of pre-greentea greentea |
| `tests.py` | implementation of `mbed test --greentea` |
| `toolchains` | API for calling the selected compiler |
| `upload_results.py` | part of pre-greentea greentea |
Copy link
Contributor

Choose a reason for hiding this comment

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

part of pre-greentea greentea

🤣

@@ -0,0 +1,48 @@
# Mbed OS Build Tools
Copy link
Contributor

Choose a reason for hiding this comment

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

😲

@cmonr
Copy link
Contributor

cmonr commented Mar 28, 2019

Would like other @ARMmbed/mbed-os-maintainers to chime in, since some of these files have been around longer than I have 😄

@cmonr cmonr requested a review from a team March 28, 2019 16:41
Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

Love it ❤️

@bridadan
Copy link
Contributor

bridadan commented Apr 1, 2019

Now you just gotta make travis happy.

@theotherjimmy
Copy link
Contributor Author

theotherjimmy commented Apr 2, 2019

Travis is unhappy because data is actually used (or at least 2 constants from it). Turns out they're not really a constant, and not really possible to use, as mbed 2 tests are not supported.

@theotherjimmy
Copy link
Contributor Author

Unit tests now pass.

Copy link
Contributor

@AnotherButler AnotherButler left a comment

Choose a reason for hiding this comment

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

Nice work on this 👍

@cmonr
Copy link
Contributor

cmonr commented Apr 2, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 2, 2019

Test run: FAILED

Summary: 1 of 9 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARMC5

Copy link
Contributor

@adbridge adbridge left a comment

Choose a reason for hiding this comment

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

LGTM

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 5, 2019

Oh no, needs quick rebase now, will restart CI then.

@jeromecoutant
Copy link
Collaborator

Hi
Is #10107 impacted ?

@theotherjimmy
Copy link
Contributor Author

@jeromecoutant I did not remove the Mbed 2 testig framework, so: no this does not affect #10107

@theotherjimmy
Copy link
Contributor Author

@0xc0170 Rebased.

@cmonr
Copy link
Contributor

cmonr commented Apr 8, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 8, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 merged commit dc1198b into ARMmbed:master Apr 9, 2019
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.

None yet

10 participants