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

chore: Add comments to SignerEntries.h #5059

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

scottschurr
Copy link
Collaborator

High Level Overview of Change

The SignerEntries class was not particularly well documented. This pull request adds a few comments to that class. The class might be confusing to some folks without the additional comments.

While I was in there I changed one parameter from std::string const& to std::string_view which might be a tiny performance boost. It avoids constructing a std::string from a char const* at the call sites.

Type of Change

  • Refactor (non-breaking change that only restructures code)
  • Documentation update

@scottschurr scottschurr requested a review from ximinez July 10, 2024 00:12
@scottschurr scottschurr added Tech Debt Non-urgent improvements Trivial Perf impact not expected Change is not expected to improve nor harm performance. labels Jul 10, 2024
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.4%. Comparing base (20707fa) to head (ad7793b).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5059     +/-   ##
=========================================
- Coverage     71.4%   71.4%   -0.0%     
=========================================
  Files          796     796             
  Lines        67031   67031             
  Branches     10865   10864      -1     
=========================================
- Hits         47832   47831      -1     
- Misses       19199   19200      +1     
Files Coverage Δ
src/xrpld/app/tx/detail/SignerEntries.cpp 68.4% <ø> (ø)
src/xrpld/app/tx/detail/SignerEntries.h 100.0% <ø> (ø)

... and 3 files with indirect coverage changes

Impacted file tree graph

@ximinez
Copy link
Collaborator

ximinez commented Jul 24, 2024

@scottschurr I agree that this PR is trivial, and I've approved it. If it's ready for merge, please add the "Passed" label or add a comment to that effect.

@scottschurr scottschurr added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jul 24, 2024
@ximinez ximinez merged commit 21a0a64 into XRPLF:develop Jul 25, 2024
19 of 20 checks passed
vlntb pushed a commit to vlntb/rippled that referenced this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. Perf impact not expected Change is not expected to improve nor harm performance. Tech Debt Non-urgent improvements Trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants