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

AmendmentTable improvements #3404

Closed

Conversation

scottschurr
Copy link
Collaborator

While working on retiring amendments I made a number of improvements to the AmendmentTable and its unit tests. Since we're not moving forward with retiring amendments, this pull request contains the improvements to the AmendmentTable and its unit tests that I believe we want to keep.

This code should look very familiar to the reviewers. It's just selected favorite highlights from the retiring amendments pull request. Thanks for looking it over again.

Copy link
Collaborator

@seelabs seelabs left a comment

Choose a reason for hiding this comment

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

Good changes all. 👍 from me.


{
std::lock_guard sl(mutex_);
amendments.reserve(amendmentMap_.size());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch on this one.

@codecov-io
Copy link

Codecov Report

Merging #3404 into develop will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3404      +/-   ##
===========================================
+ Coverage    70.44%   70.45%   +0.01%     
===========================================
  Files          682      682              
  Lines        54392    54389       -3     
===========================================
+ Hits         38315    38321       +6     
+ Misses       16077    16068       -9     
Impacted Files Coverage Δ
src/ripple/app/misc/AmendmentTable.h 100.00% <ø> (ø)
src/ripple/app/misc/impl/AmendmentTable.cpp 98.17% <100.00%> (+4.02%) ⬆️
src/ripple/rpc/handlers/Feature1.cpp 96.15% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9771210...b7fdb29. Read the comment docs.

{
// Forward to the const version of get.
return const_cast<AmendmentState*>(
std::as_const(*this).get(amendmentHash, sl));
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this now? std::as_const? Neat.

This was referenced May 21, 2020
Copy link
Contributor

@pwang200 pwang200 left a comment

Choose a reason for hiding this comment

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

Looks good!

@scottschurr scottschurr added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 21, 2020
@manojsdoshi manojsdoshi mentioned this pull request May 27, 2020
@scottschurr scottschurr deleted the amendment-table-const-correct branch June 22, 2020 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants