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.

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

Refactoring \platform directory #13298

Merged
merged 5 commits into from
Aug 5, 2020
Merged

Refactoring \platform directory #13298

merged 5 commits into from
Aug 5, 2020

Conversation

ashok-rao
Copy link
Contributor

@ashok-rao ashok-rao commented Jul 15, 2020 •
edited by evedon
Loading

Summary of changes

Refactoring platform as per directory structure proposal.
Mbed OS will soon be changing directory structure to the below:

[component name]
├── mbed_lib.json 
├── CMakeList.txt
├── README.md
├── include 
│   └── [component name]
│       └── internal              
├── source
└── tests
    ├── <framework>  
    │   └── <test suite one> 
    ├── UNITTESTS               
    │   └── <unit test suite one>   
    └── TESTS       
        └── <greentea test suite one>

This PR implements the above new directory structure for platform.

Impact of changes

-NA-

Migration actions required

-NA-

Documentation

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)
[x] Tests / results supplied as part of this PR

Build successes:

  • NUCLEO_F429ZI::GCC_ARM::MBED-BUILD
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-ATOMIC
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-CIRCULARBUFFER
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-CRASH_REPORTING
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-CRITICAL_SECTION
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-ERROR_HANDLING
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-FILEHANDLE
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-MINIMAL-PRINTF
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-SHAREDPTR
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-SINGLETONPTR
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-STREAM
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-SYSTEM_RESET
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-TRANSACTION
  • NUCLEO_F429ZI::GCC_ARM::MBED-OS-PLATFORM-TESTS-TESTS-MBED_PLATFORM-WAIT_NS

Reviewers

@0xc0170 @LDong-Arm @evedon

@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 15, 2020
@ciarmcom ciarmcom requested review from 0xc0170, evedon, LDong-Arm and a team July 15, 2020 17:00
@ciarmcom
Copy link
Member

@ashok-rao, thank you for your changes.
@LDong-Arm @0xc0170 @evedon @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

@mergify
Copy link

mergify bot commented Jul 15, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@LDong-Arm
Copy link
Contributor

The current change looks good. Will review again once the remaining part is done.

0xc0170
0xc0170 previously requested changes Jul 16, 2020
View reviewed changes
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.

Extend the commit msg , refactor should not be one liner.

evedon
evedon previously approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@evedon evedon left a comment

Choose a reason for hiding this comment

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

The code change is fine but you need to update the PR description to indicate that you did not include UNITTESTS in this PR (this will be done in a subsequent PR)

@mergify mergify bot dismissed stale reviews from 0xc0170 and evedon July 17, 2020 12:24

Pull request has been modified.

@ashok-rao
Copy link
Contributor Author

Extend the commit msg , refactor should not be one liner.

Thanks @0xc0170 . Now done. Force pushed with new commit message.

@ashok-rao
Copy link
Contributor Author

The code change is fine but you need to update the PR description to indicate that you did not include UNITTESTS in this PR (this will be done in a subsequent PR)

Thanks @evedon . Now done.

@ashok-rao
Copy link
Contributor Author

@0xc0170 .. I just pushed a new commit with new mbed_version.h file in this PR. Can you please have a look? Thanks.

@ashok-rao ashok-rao requested review from 0xc0170 and evedon July 22, 2020 08:53
0xc0170
0xc0170 previously approved these changes Jul 22, 2020
View reviewed changes
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 22, 2020

CI started

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

0xc0170 commented Jul 22, 2020

CI started

0xc0170
0xc0170 previously approved these changes Jul 28, 2020
View reviewed changes
@mbed-ci
Copy link

mbed-ci commented Jul 28, 2020

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@mergify mergify bot added needs: work and removed needs: CI labels Jul 28, 2020
@gpsimenos
Copy link
Contributor

This appears to be the same failure as in #13356 (comment).

@gpsimenos gpsimenos mentioned this pull request Jul 28, 2020
Merged
@mergify
Copy link

mergify bot commented Jul 29, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 29, 2020

Once rebased, CI run - all should be fine or not?

@gpsimenos
Copy link
Contributor

Not yet, greentea tests will fail. We are working on a solution with @jamesbeyond which will probably come in a separate PR to be merged before this one and #13356.

@gpsimenos
Copy link
Contributor

@ashok-rao You need to update the description above, since the unit tests are now being moved in this PR.

evedon
evedon previously approved these changes Jul 30, 2020
View reviewed changes
@gpsimenos
Copy link
Contributor

Not yet, greentea tests will fail. We are working on a solution with @jamesbeyond which will probably come in a separate PR to be merged before this one and #13356.

There will be no separate PR, we agreed on the temporary solution of duplicating some host test scripts inside each component. After @ashok-rao makes that change, CI should pass.

@mergify mergify bot dismissed stale reviews from 0xc0170 and evedon July 31, 2020 14:16

Pull request has been modified.

@ashok-rao
Copy link
Contributor Author

@0xc0170 or @adbridge .. ready for CI one-more-time, now with the necessary host-tests pushed. Thanks.

gpsimenos
gpsimenos approved these changes Aug 3, 2020
View reviewed changes
Copy link
Contributor

@gpsimenos gpsimenos left a comment

Choose a reason for hiding this comment

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

LGTM

@adbridge
Copy link
Contributor

adbridge commented Aug 4, 2020

CI started

@adbridge
Copy link
Contributor

adbridge commented Aug 4, 2020

mmm moving mbed_version.h breaks the release script ! I'll have to update that once this gets merged!

@mbed-ci
Copy link

mbed-ci commented Aug 4, 2020

Test run: SUCCESS

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

@ashok-rao
Copy link
Contributor Author

@adbridge.. when can this one be merged? I see it still needs some approval, but this has already been approved by the core team. Thanks.

@adbridge adbridge merged commit c85b76a into ARMmbed:master Aug 5, 2020
@mbedmain mbedmain added release-version: 6.2.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adbridge adbridge adbridge approved these changes

@gpsimenos gpsimenos gpsimenos approved these changes

@LDong-Arm LDong-Arm LDong-Arm approved these changes

@0xc0170 0xc0170 0xc0170 left review comments

@evedon evedon evedon left review comments

Assignees
No one assigned
Labels
release-version: 6.2.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ashok-rao @ciarmcom @LDong-Arm @0xc0170 @mbed-ci @evedon @gpsimenos @adbridge @mbedmain