Skip to content

Conversation

Umiiii
Copy link
Contributor

@Umiiii Umiiii commented Apr 23, 2024

Add missing comparison for TODO comments in mempool_packages.py

Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 23, 2024

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@DrahtBot DrahtBot added the Tests label Apr 23, 2024
Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

Thanks for the pull, please squash your commits

@Umiiii
Copy link
Contributor Author

Umiiii commented Apr 23, 2024

Thanks for the pull, please squash your commits

squashed.

assert_equal(mempool[tx]['unbroadcast'], False)

# TODO: test ancestor size limits
# Check more detailed check of node1's mempool
Copy link
Member

Choose a reason for hiding this comment

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

these comments are not needed(and indented wrong); I think they can just all be removed since the test is self-explanatory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please clarify what you meant by "indented wrong"? I believe these lines are intended to follow the same indentation pattern as the rest of the file. If there's a specific indentation style that should be applied here, I'd appreciate your guidance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I agreed with you. Those lines are self explanatory. I removed the comments.

@Umiiii
Copy link
Contributor Author

Umiiii commented Apr 24, 2024

@glozow could you review again and see if it meets the requirements now ..?

@Umiiii Umiiii closed this Apr 24, 2024
achow101 added a commit that referenced this pull request May 10, 2024
…oolPackagesTest

e912717 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi)

Pull request description:

  #29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed.

  Add missing comparison for TODO comments in `mempool_packages.py`

  Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800   ,  so I removed the todo for those two size limits.

ACKs for top commit:
  kevkevinpal:
    ACK [e912717](e912717)
  achow101:
    ACK e912717
  alfonsoromanz:
    Tested ACK e912717. The code looks good to me and the test execution is successful.
  rkrux:
    tACK [e912717](e912717)

Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 25, 2024
…in MempoolPackagesTest

e912717 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi)

Pull request description:

  bitcoin#29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed.

  Add missing comparison for TODO comments in `mempool_packages.py`

  Also, notice that the ancestor size limits and descendant size limits actually implemented in bitcoin#21800   ,  so I removed the todo for those two size limits.

ACKs for top commit:
  kevkevinpal:
    ACK [e912717](bitcoin@e912717)
  achow101:
    ACK e912717
  alfonsoromanz:
    Tested ACK e912717. The code looks good to me and the test execution is successful.
  rkrux:
    tACK [e912717](bitcoin@e912717)

Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 26, 2024
…in MempoolPackagesTest

e912717 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi)

Pull request description:

  bitcoin#29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed.

  Add missing comparison for TODO comments in `mempool_packages.py`

  Also, notice that the ancestor size limits and descendant size limits actually implemented in bitcoin#21800   ,  so I removed the todo for those two size limits.

ACKs for top commit:
  kevkevinpal:
    ACK [e912717](bitcoin@e912717)
  achow101:
    ACK e912717
  alfonsoromanz:
    Tested ACK e912717. The code looks good to me and the test execution is successful.
  rkrux:
    tACK [e912717](bitcoin@e912717)

Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
@bitcoin bitcoin locked and limited conversation to collaborators Apr 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants