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

move get_name_puzzle_conditions() into test utils #18839

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Nov 8, 2024

It's only used by tests now.

Purpose:

reduce the amount of production code.

Current Behavior:

get_name_puzzle_conditions() is only used by tests but live in chia.full_node.mempool_check_conditions

New Behavior:

get_name_puzzle_conditions() is only used by tests and live in chia._tests.util.get_name_puzzle_conditions

Testing Notes:

@arvidn arvidn added Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes Tests Changes to tests labels Nov 8, 2024
@arvidn arvidn force-pushed the move-get_name_puzzle_conditions branch 2 times, most recently from ee33718 to cbdd10c Compare November 8, 2024 10:02
@arvidn arvidn requested a review from AmineKhaldi November 8, 2024 10:25
@arvidn arvidn marked this pull request as ready for review November 8, 2024 10:26
@arvidn arvidn requested a review from a team as a code owner November 8, 2024 10:26
Copy link
Contributor

@AmineKhaldi AmineKhaldi left a comment

Choose a reason for hiding this comment

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

I would move it to chia/_tests/util/run_block.py.

@arvidn
Copy link
Contributor Author

arvidn commented Nov 8, 2024

I would move it to chia/_tests/util/run_block.py.

why?

@AmineKhaldi
Copy link
Contributor

I would move it to chia/_tests/util/run_block.py.

why?

It just seemed unnecessary to give it its own module while we have a test utils one about functions related to running blocks. Up to you.

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Nov 12, 2024
@arvidn arvidn force-pushed the move-get_name_puzzle_conditions branch from cbdd10c to 471bbfa Compare November 19, 2024 09:12
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Nov 19, 2024
Copy link
Contributor

File Coverage Missing Lines
chia/_tests/util/get_name_puzzle_conditions.py 88.9% lines 50-52
Total Missing Coverage
42 lines 3 lines 92%

@arvidn
Copy link
Contributor Author

arvidn commented Nov 20, 2024

@AmineKhaldi I don't think there's a cost of having one more file, but separating them may avoid module cycles

@arvidn arvidn requested a review from AmineKhaldi November 20, 2024 11:44
@arvidn arvidn requested a review from emlowe November 20, 2024 13:20
@pmaslana pmaslana merged commit 84dc738 into main Nov 20, 2024
361 of 362 checks passed
@pmaslana pmaslana deleted the move-get_name_puzzle_conditions branch November 20, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes Tests Changes to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants