[Improvement] Have a matching commenting style of imported libs and drivers

TheThingsIndustries / generic-node-se

Generic Node Sensor Edition

https://www.genericnode.com

Other

110 stars 31 forks source link

[Improvement] Have a matching commenting style of imported libs and drivers #38

Open elsalahy opened 4 years ago

elsalahy commented 4 years ago

Summary:

In case in the future if we need to generate a doxygen reference, we will need to ensure all libs and drivers have similar commenting style to ensure the coherency and consistency of the auto generated reference doc.

Why do we need this?

To have a better overall documentation.

What is already there? What do you see now?

Our source code has a simple commenting stye.

What is missing? What do you want to see?

Having all the imported libs/drivers with similar style.

How do you propose to implement this?

Refactor the commenting style if we feel the effort is justified.

Environment:

Baremetal.

Acceptance Criteria:

...

What can you do yourself and what do you need help with?

ALL, will be postponed until we make a decision if it's necessary to do this.

elsalahy commented 4 years ago

To keep Track:

SHTC3 lib needs minor modifications in order to have a matching commenting style.

elsalahy commented 3 years ago

@marnixcro please consider undertaking this?

mcserved commented 3 years ago

@elsalahy I did some work for this already but was wondering about which ones should actually be changed. I made an overview of what I thing is lacking in each lib:

[x] ATEC608A: seems fine
[ ] BUZZER: has no documentation
[ ] FreeRTOS-Kernel: no doxygen
[x] FreeRTOS-LoRaWAN: seems fine
[x] GNSE_BSP: seems fine
[x] GNSE_HAL: seems fine
[ ] GNSE_TRACER: tiny_printf misses some doxygen formatting (primarily by not using \brief)
[x] LIS2DH12: seems fine
[x] MCU_FLASH: seems fine
[ ] MX25R1635: missing doxygen
[ ] SHTC3: missing doxygen style in many cases, has lots of unnecessary info
[ ] SPIFFS: missing doxygen
[ ] STM32WLxx_LoRaWAN: inconsistent between folders, mostly missing doxygen

My current changes only target Buzzer, MX25R1635, SHTC3, and Spiffs.

Would changing all of these be alright or should some bigger libraries (STM32WLxx_LoRaWAN or FreeRTOS for instance) be left alone for now?

elsalahy commented 3 years ago

@marnixcro great overview, let's start small and target libraries maintained by us We shouldn't fix/update any library that is OS or LoRaWAN related.

The changes should target the comments only to match our guidelines. Also it's good to know that this is a technical debt issue and it's extremely low priority as we have more urgent functional issues.

So please consider doing this only when you have nothing else on your plate.

elsalahy commented 3 years ago

@marnixcro have you done any development for this issue, if yes, please share the branch.

mcserved commented 3 years ago

@marnixcro have you done any development for this issue, if yes, please share the branch.

I pushed it to docs/docstyle-match