add pylint to catch unused args
#2803
Conversation
| @@ -1832,7 +1832,7 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: | |||
| ##### Deposits | |||
|
|
|||
| ```python | |||
| def get_validator_from_deposit(state: BeaconState, deposit: Deposit) -> Validator: | |||
| def get_validator_from_deposit(deposit: Deposit) -> Validator: | |||
There was a problem hiding this comment.
this does change the phase0 specs BUT no substantive change to functionality, just a code cleanup
| @@ -514,7 +514,7 @@ def get_pow_block(hash: Bytes32) -> Optional[PowBlock]: | |||
| return PowBlock(block_hash=hash, parent_hash=Bytes32(), total_difficulty=uint256(0)) | |||
|
|
|||
|
|
|||
| def get_execution_state(execution_state_root: Bytes32) -> ExecutionState: | |||
| def get_execution_state(_execution_state_root: Bytes32) -> ExecutionState: | |||
There was a problem hiding this comment.
this satisfy pylint by indicating it is intended to be unused, which seems like a fine solution to me while we are still iterating on the bellatrix auxillary code
| lint: pyspec | ||
| . venv/bin/activate; cd $(PY_SPEC_DIR); \ | ||
| flake8 --config $(LINTER_CONFIG_FILE) ./eth2spec \ | ||
| && pylint --disable=all --enable unused-argument ./eth2spec/phase0 ./eth2spec/altair ./eth2spec/bellatrix \ |
There was a problem hiding this comment.
pylint adds a bunch of stuff we are not following wrt style, so instead we disable=all and then only enable the thing we want, which is the unused-argument linter
| @@ -521,7 +521,7 @@ def process_deposit(state: BeaconState, deposit: Deposit) -> None: | |||
| signing_root = compute_signing_root(deposit_message, domain) | |||
| # Initialize validator if the deposit signature is valid | |||
| if bls.Verify(pubkey, signing_root, deposit.data.signature): | |||
| state.validators.append(get_validator_from_deposit(state, deposit)) | |||
| state.validators.append(get_validator_from_deposit(deposit)) | |||
There was a problem hiding this comment.
update from phase0 as well
this utility function is simple enough that we would basically just be writing the implementation which doesn't add a lot of extra assurance imo there is also plenty of transitive test coverage via things that use the |
fixes #2761.