Doxygen correction #9326
Doxygen correction #9326
Conversation
|
@hasnainvirk, thank you for your changes. |
|
Afraid I don't really know this structural stuff in doxygen well enough to comment on it. |
|
@melwee01 There are no structural changes here. Adding to a group merely forces the doxygen for these files to appear under Class Hierarchy rather than data structures. |
There was a problem hiding this comment.
Few questions:
- Could you explain the reason of the change ? Entities we wanted in the NFC group where enclosed into an
addtogroupcommand; now it includes everything that is declared in the file (including namespaces) which is not always what we want. - What description comment doxygen pickup when it encounters these addtogroup statement ?
/** @addtogroup NFC
* NFC Controller
* @{
*/
// then
/** @addtogroup NFC
* NFC EEPROM
* @{
*/There is two very different comment for the same group: NFC Controller and NFC EEPROM. What is rendered ?
| @@ -14,6 +14,10 @@ | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| /** @addtogroup NFC | |||
There was a problem hiding this comment.
The previous addtogroup command hasn't been removed.
|
@pan- It says that the following file is under a group NFC and the module description is given in the next line. I am not sure why the previous format did not get parsed correctly by doxygen and that's why the documentation of NFC content went to the data structure section. Using the new format places it correctly under class hierarchy section. You can test it locally |
|
@ARMmbed/mbed-os-pan Pls review |
There was a problem hiding this comment.
I tried it locally and the results are not good:
- There is now two NFC modules:
NfcandNFC. - Data structures impacted by this PR does not appear in their module.
- Additional text added in each addtogroup directive yield nonsense.
To document files, use the @file directive must be used.
|
@hasnainvirk Thoughts? |
hasnainvirk
force-pushed
the
nfc_doxy
branch
from
7677c08 to
83e4bf8
Compare
features/nfc/nfc/NFCController.h
Outdated
| @@ -1,6 +1,8 @@ | |||
| /* mbed Microcontroller Library | |||
| * Copyright (c) 2018 ARM Limited | |||
| * | |||
| * @file NFCController.h | |||
There was a problem hiding this comment.
That is not going to do anything as it is not part of a Doxygen comment. The same is true for other headers modified.
There was a problem hiding this comment.
Interesting, I have seen many examples where file description is added alongwith license text. I can try to add a separate doxy section for @file.
There was a problem hiding this comment.
@hasnainvirk I'm not saying @file isn't a valid directive. I'm just pointing out it should be part of a Doxygen comment. A doxygen comment starts with /**, /*!, /// or //!. In this case the licence documentation block starts with /*
There was a problem hiding this comment.
Adjusted the license block instead.
There was a problem hiding this comment.
@pan- Yeah, figured that out while you were writing your message :)
Adding to proper group so that the API doxygen appears into the class hierarchy group rather than data strutures.
hasnainvirk
force-pushed
the
nfc_doxy
branch
from
83e4bf8 to
2670eaf
Compare
|
CI started |
Test run: SUCCESSSummary: 12 of 12 test jobs passed |
|
@ARMmbed/mbed-docs Heads up, doxygen change coming into 5.11. |
|
@cmonr Why did you merge it ? It hasn't been approved by the pan team. If you want to add a documentation block about the file the pattern must be: |
|
@hasnainvirk can you please fix it? |
|
@pan- Does it matter ? I am not 100% but it shouldn't matter. Most of the public code would have \file \detailes \brief in the license text. |
|
I'm sure it does matters if you look at the rendered documentation:
Does that looks good ? |
|
My mistake @pan- |
Description
Adding to proper group so that the API doxygen appears into the class
hierarchy group rather than data strutures.
Pull request type
Reviewers
@melwee01 @AnotherButler