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

Clarify default defines #1106

Merged
merged 1 commit into from Apr 10, 2018

Conversation

Projects
None yet
2 participants
@mfalkvidd
Copy link
Contributor

mfalkvidd commented Apr 7, 2018

Moved example values to the @ brief portion of Doxygen,
so the default (empty/undefined) values are visible
in Doxygen.

@mfalkvidd mfalkvidd requested review from fallberg and bblacey Apr 7, 2018

@mfalkvidd

This comment has been minimized.

Copy link
Contributor Author

mfalkvidd commented Apr 7, 2018

Before this change:
image
after this change:
image

@fallberg
Copy link
Contributor

fallberg left a comment

When a doxygen entry is supposed to go with a section (@ brief in this case) it is more clear to indent the addition to clarify this. Leaving it on a new line can give the incorrect impression that it will not be part of the section or it will be line broken in the section (which it won't). See line 1519 as reference.
I suggest we add the examples using something like this:

 * @brief Define this if RS485 is connected to a hardware serial port.\n
 *        Example: \c \#define MY_RS485_HWSERIAL Serial1

Also, since the entire example is "code" it might be more clear to the user to have the entire #define expression as fixed width or syntax highlighted using @ code or similar.
This might (I have not tested) work:

 * @brief Define this if RS485 is connected to a hardware serial port. Example:
 *        @code #define MY_RS485_HWSERIAL Serial1 @endcode

One could also argue that the examples do not belong in the @ brief, since it is supposed to give the very condensed description of things. Having examples there bloats it significantly, and for most of the cases the examples are quite self explanatory.

@mfalkvidd

This comment has been minimized.

Copy link
Contributor Author

mfalkvidd commented Apr 9, 2018

@fallberg how about this? Brief is kept short and entire example is in monospace.
image
Code:

/**
 * @def MY_SECURITY_SIMPLE_PASSWD
 * @brief Enables SW backed signing functionality and encryption functionality in library and uses
 *        provided password as key.
 *
 * Example: @code #define MY_SECURITY_SIMPLE_PASSWD "MyInsecurePassword" @endcode
 *
 * For details on the effects, see the references.
 * @see MY_SIGNING_SIMPLE_PASSWD, MY_ENCRYPTION_SIMPLE_PASSWD
 */
//#define MY_SECURITY_SIMPLE_PASSWD "MyInsecurePassword"

Clarify default defines
Moved example values to the @brief portion of Doxygen,
so the default (empty/undefined) values are visible
in Doxygen.

@mfalkvidd mfalkvidd force-pushed the mfalkvidd:Clarify-default-defines branch from f051b80 to 894ca3a Apr 10, 2018

@mfalkvidd mfalkvidd merged commit a842986 into mysensors:development Apr 10, 2018

24 checks passed

Toll gate Pass
Details
Toll gate (Arduino Mega - Examples) Pass
Details
Toll gate (Arduino Mega - Tests) Pass
Details
Toll gate (Arduino Uno - Examples) Pass
Details
Toll gate (Arduino Uno - Tests) Pass
Details
Toll gate (Code analysis - Cppcheck) Pass
Details
Toll gate (Documentation) Pass
Details
Toll gate (ESP32 - Tests) Pass
Details
Toll gate (ESP8266 - Examples) Pass
Details
Toll gate (ESP8266 - Tests) Pass
Details
Toll gate (Gitler) Pass
Details
Toll gate (Linux builds - Ethernet GW) Pass
Details
Toll gate (Linux builds - MQTT GW) Pass
Details
Toll gate (Linux builds - Serial GW) Pass
Details
Toll gate (MySensorsGW - Examples) Pass
Details
Toll gate (MySensorsGW - Tests) Pass
Details
Toll gate (MySensorsMicro - Examples) Pass
Details
Toll gate (MySensorsMicro - Tests) Pass
Details
Toll gate (STM32F1 - Tests) Pass
Details
Toll gate (nRF5 - Examples) Pass
Details
Toll gate (nRF5 - Tests) Pass
Details
Toll gate (nRF51822 - Tests) Pass
Details
Toll gate (nRF52832 - Tests) Pass
Details
clahub All contributors have signed the Contributor License Agreement.
Details

@mfalkvidd mfalkvidd deleted the mfalkvidd:Clarify-default-defines branch Apr 10, 2018

fallberg added a commit that referenced this pull request Jun 18, 2018

Clarify default defines (#1106)
Moved example values after the @brief portion of Doxygen,
so the default (empty/undefined) values are visible
in Doxygen.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.