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

refactor: Rename local variable to distinguish it from type alias #26624

Merged
merged 1 commit into from Dec 5, 2022

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Dec 2, 2022

The txiter type alias is declared in the txmempool.h:

using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;

The `txiter` type alias is declared in the `txmempool.h`.
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 2, 2022

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK stickies-v, vasild, jarolrod

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

ACK 1984db1

Straightforward change, better to not have naming conflicts.

Comment on lines +2294 to +2296
auto tx_iter = m_mempool.GetIter(tx->GetHash());
if (tx_iter) {
const CTxMemPoolEntry::Parents& parents = (*tx_iter)->GetMemPoolParentsConst();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, while touching:

Suggested change
auto tx_iter = m_mempool.GetIter(tx->GetHash());
if (tx_iter) {
const CTxMemPoolEntry::Parents& parents = (*tx_iter)->GetMemPoolParentsConst();
auto tx_iter{m_mempool.GetIter(tx->GetHash())};
if (tx_iter) {
const CTxMemPoolEntry::Parents& parents{(*tx_iter)->GetMemPoolParentsConst()};

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 1984db1

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK 1984db1

I have reviewed the code and agree it can be merged.

@glozow glozow merged commit 8b79686 into bitcoin:master Dec 5, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 5, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants