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

Address comments on workaround for Mbed OS 2 CI build after Public #11114

Merged

Conversation

hugueskamba
Copy link
Collaborator

Description

The comments addressed can be found here

Pull request type

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

Reviewers

@madchutney @evedon

Release Notes

@ciarmcom
Copy link
Member

@hugueskamba, thank you for your changes.
@madchutney @evedon @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested review from evedon, madchutney and a team July 27, 2019 03:00
Modify compilation API to provide a list of paths to exclude from
the build.
Copy link
Contributor

@madchutney madchutney left a comment

Choose a reason for hiding this comment

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

Thanks for adding the unit tests, please don't force push as it makes it harder to review subsequent changes, thanks.

tools/toolchains/mbed_toolchain.py Outdated Show resolved Hide resolved
tools/toolchains/mbed_toolchain.py Outdated Show resolved Hide resolved
tools/test/build_api/build_api_test.py Show resolved Hide resolved
tools/test/build_api/build_api_test.py Outdated Show resolved Hide resolved
@hugueskamba hugueskamba force-pushed the hk-fix-mbed-os-2-build branch 2 times, most recently from 0629b9b to 79d119f Compare July 29, 2019 13:22
* `_exclude_files_from_build` becomes a static method
* Replace ternary expression with simple  `if/else` statement

Co-Authored-By: Graham Hammond <graham.hammond@arm.com>
@hugueskamba
Copy link
Collaborator Author

Thanks for adding the unit tests, please don't force push as it makes it harder to review subsequent changes, thanks.

Old habits die hard...Yes I will avoid doing that.

@evedon
Copy link
Contributor

evedon commented Jul 30, 2019

CI failing due to a different issue (trollius).
Merging PR to the feature branch.

@evedon evedon merged commit 9f44765 into ARMmbed:feature-public-headers Jul 30, 2019
@hugueskamba hugueskamba deleted the hk-fix-mbed-os-2-build branch July 30, 2019 10:03
evedon pushed a commit that referenced this pull request Aug 1, 2019
…11114)


* Modify compilation API to provide a list of paths to exclude from the build.
* `_exclude_files_from_build` becomes a static method
* Replace ternary expression with simple  `if/else` statement
* Make unit test case for dirs exclusion independent of system files
evedon pushed a commit that referenced this pull request Aug 2, 2019
…11114)


* Modify compilation API to provide a list of paths to exclude from the build.
* `_exclude_files_from_build` becomes a static method
* Replace ternary expression with simple  `if/else` statement
* Make unit test case for dirs exclusion independent of system files
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

4 participants