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.

Sign up for GitHub

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

Jump to bottom

[CHIA-1732] Add Ruff specific linting rules #18845

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[CHIA-1732] Add Ruff specific linting rules #18845

wants to merge 2 commits into from
+279 −255

Conversation

Quexington
Copy link
Contributor

No description provided.

@Quexington Quexington changed the title Add Ruff specific linting rules [CHIA-1732] Add Ruff specific linting rules Nov 8, 2024
@Quexington Quexington added Added Required label for PR that categorizes merge commit message as "Added" for changelog Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes labels Nov 8, 2024
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Nov 12, 2024
@Quexington Quexington force-pushed the quex.enable_ruff_specific_rules branch from 8f50032 to 53803e3 Compare November 13, 2024 22:43
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Nov 13, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Nov 13, 2024
@Chia-Network Chia-Network deleted a comment from github-actions bot Nov 13, 2024
Quexington
Quexington commented Nov 13, 2024
View reviewed changes
chia/_tests/cmds/test_click_types.py
Comment on lines +41 to +43
@dataclass
class FakeContext:
obj: dict[Any, Any] = {}

def __init__(self, obj: dict[Any, Any]):
self.obj = obj
obj: dict[Any, Any] = field(default_factory=dict)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ruff didn't automatically make this fix, this was my response to a warning about the mutable default

Quexington
Quexington commented Nov 13, 2024
View reviewed changes
chia/_tests/core/mempool/test_mempool_manager.py
@@ -245,7 +246,7 @@ class TestCheckTimeLocks:
coinbase=False,
timestamp=COIN_TIMESTAMP,
)
REMOVALS: dict[bytes32, CoinRecord] = {TEST_COIN.name(): COIN_RECORD}
REMOVALS: dict[bytes32, CoinRecord] = dataclasses.field(default_factory=lambda: {TEST_COIN.name(): COIN_RECORD}) # type: ignore[name-defined] # noqa: F821
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, this is me addressing a concern about the mutable default. The linters still don't like it because they don't recognize COIN_RECORD as a valid variable, but it definitely passes the tests 🤷

Quexington
Quexington commented Nov 13, 2024
View reviewed changes
pyproject.toml
@@ -142,5 +142,6 @@ requires = ["poetry-core>=1.0.0", "poetry-dynamic-versioning"]
build-backend = "poetry_dynamic_versioning.backend"

[project]
name = "chia-blockchain"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure of any ripple effects of this, might be another conflict with poetry like the version.

@Quexington Quexington marked this pull request as ready for review November 13, 2024 22:50
@Quexington Quexington requested a review from a team as a code owner November 13, 2024 22:50
@github-actions GitHub Actions
Copy link
Contributor

File Coverage Missing Lines
chia/_tests/process_junit.py 0.0% lines 209, 286
chia/_tests/wallet/rpc/test_dl_wallet_rpc.py 50.0% lines 122
chia/cmds/check_wallet_db.py 50.0% lines 345
chia/farmer/farmer.py 50.0% lines 201
chia/full_node/full_node.py 66.7% lines 283, 293
chia/full_node/mempool_manager.py 50.0% lines 251
chia/introducer/introducer.py 0.0% lines 109
chia/pools/pool_wallet.py 0.0% lines 414
chia/pools/pool_wallet_info.py 0.0% lines 96
chia/seeder/dns_server.py 0.0% lines 83, 195, 477
chia/server/server.py 0.0% lines 505
chia/server/ws_connection.py 62.5% lines 670, 718, 723
chia/util/profiler.py 0.0% lines 138
chia/wallet/wallet_node.py 50.0% lines 429, 432
Total Missing Coverage
176 lines 21 lines 88%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@altendky altendky Awaiting requested review from altendky

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Added Required label for PR that categorizes merge commit message as "Added" for changelog coverage-diff Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Quexington