Skip to content

style: Unify style for all Doxygen comments#3142

Open
mathbunnyru wants to merge 4 commits into
XRPLF:developfrom
mathbunnyru:doxygen_style_comments
Open

style: Unify style for all Doxygen comments#3142
mathbunnyru wants to merge 4 commits into
XRPLF:developfrom
mathbunnyru:doxygen_style_comments

Conversation

@mathbunnyru

Copy link
Copy Markdown
Collaborator

No description provided.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.65%. Comparing base (52d28f8) to head (23a08e7).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3142   +/-   ##
========================================
  Coverage    82.65%   82.65%           
========================================
  Files          398      398           
  Lines        16118    16118           
  Branches      8443     8443           
========================================
  Hits         13323    13323           
  Misses        1674     1674           
  Partials      1121     1121           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

std::string
path; /**< path to the file to load cache from on start and save cache to on shutdown */
uint32_t maxAge = 5000; /**< max difference between latest sequence in cache file and DB */
path; ///< path to the file to load cache from on start and save cache to on shutdown

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Didn't we prefer /**< for these?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In rippled, there are actually more places with ///<.
In clio, there are already many places with ///< too.

So, overall - incosistent.

Both these styles are in their own way good:

  • /**< uses stars more similar to regular doxygen comments we have
  • ///< doesn't require you to clutter the code on the right side, just as our regular doxygen comments

Ans since inline doxygen comments are a bit different, I think it's ok to use / instead of *.
In the end, it's all getting more consistent, and I like not having to worry to close your comment, so I suggest we do it this way.

* @brief Calls callback on the oracle ledger entry
If the oracle entry does not contains the price pair, search up to three previous metadata
objects. Stops early if the callback returns true.
* If the oracle entry does not contains the price pair, search up to three previous metadata

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: We usually try to separate brief from detail by a space line

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Comment thread src/util/Channel.hpp Outdated
Multi /**< Multiple Senders can exist (copyable). Uses shared_ptr<Guard> for shared ownership.
*/
Single, ///< Only one Sender can exist (non-copyable). Uses direct Guard ownership for zero
///< overhead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it because of this? Alignment looks off now

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, and I think it reads better than before

@mathbunnyru mathbunnyru requested a review from godexsoft July 11, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants