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

Upstream PR #11073 review request changes #11135

Merged

Conversation

hugueskamba
Copy link
Collaborator

@hugueskamba hugueskamba commented Jul 31, 2019

Description

Addressing comments from: #11073 (review)

Pull request type

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

Reviewers

@evedon

Release Notes

Greetea test result for USB using the command:
mbed test -t arm -m k64f -n tests-usb_device-*
is
mbedgt: test case results: 1 FAIL / 30 SKIPPED / 3 OK / 2 ERROR
The same result is obtained with the master branch

@ciarmcom ciarmcom requested review from evedon and a team July 31, 2019 15:00
@ciarmcom
Copy link
Member

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

drivers/Ethernet.h Outdated Show resolved Hide resolved
drivers/Ethernet.h Outdated Show resolved Hide resolved
drivers/source/usb/mbed_lib.json Show resolved Hide resolved
platform/CallChain.h Show resolved Hide resolved
* Remove `#warning` directive
* Add deprecation warning for all class methods
* Replace Doxygen `@warning` with `@deprecated`
@@ -28,11 +28,9 @@
#include <cstdio>

namespace mbed {
/** \ingroup mbed-os-public */
Copy link
Member

Choose a reason for hiding this comment

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

Why did we drop the public?

Copy link
Collaborator Author

@hugueskamba hugueskamba Aug 1, 2019

Choose a reason for hiding this comment

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

It is still in the Public API. See line 33. If you are able to, have a look at the generated Doxygen output.

Copy link
Member

Choose a reason for hiding this comment

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

yeah but why change of groups? We used to have the
Public -> [Drivers, RTOS, ...]
Private -> [...]
is this change affecting the group tree?

Copy link
Collaborator Author

@hugueskamba hugueskamba Aug 1, 2019

Choose a reason for hiding this comment

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

The generated Doxygen ouptut has not changed. This change simply makes the Doxygen comment of this file in line with other files under drivers.
drivers-public-api is defined as being in Public API->Drivers on the Doxygen output.

@@ -23,8 +23,7 @@

#include "events/mbed_shared_queues.h"

/** \ingroup mbed-os-internal */
/** \addtogroup events-internal-api */
/** \addtogroup events-public-api */
Copy link
Member

Choose a reason for hiding this comment

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

That's 180deg change, why?

Copy link
Collaborator Author

@hugueskamba hugueskamba Aug 1, 2019

Choose a reason for hiding this comment

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

The file is not in mbed-os/events/internal but mbed-os/events/ that's where the events Public headers are located.

@evedon evedon merged commit c1c584c into ARMmbed:feature-public-headers Aug 1, 2019
@hugueskamba hugueskamba deleted the hk-feature-public-headers branch August 1, 2019 10:12
evedon pushed a commit that referenced this pull request Aug 1, 2019
* Modify Doxygen grouping of `drivers` Public/Internal APIs
* Correct classification of `mbed_events.h`
* Amend name of Doxygen group containing Device Key API
* Classify `CallChain.h` as public API and relocate file
* Remove Doxygen group from `equeue_platform.h` as it has no Doxygen compliant documentation
* Move USB target specific code back to `usb/device/targets`
evedon pushed a commit that referenced this pull request Aug 2, 2019
* Modify Doxygen grouping of `drivers` Public/Internal APIs
* Correct classification of `mbed_events.h`
* Amend name of Doxygen group containing Device Key API
* Classify `CallChain.h` as public API and relocate file
* Remove Doxygen group from `equeue_platform.h` as it has no Doxygen compliant documentation
* Move USB target specific code back to `usb/device/targets`
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.

4 participants