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

Lint ForbidKeysRemoveCall #927

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Conversation

eagr
Copy link
Contributor

@eagr eagr commented Nov 6, 2024

Description

Add a custom lint to forbid Keys::<T>::remove()

Related Issue(s)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe): lint

@eagr eagr requested a review from unconst as a code owner November 6, 2024 18:01
@eagr
Copy link
Contributor Author

eagr commented Nov 6, 2024

first, let's see if the lint complains about the usage in replace_neuron, then I'll disable the lint in it

@sam0x17 sam0x17 added the no-spec-version-bump PR does not contain changes that requires bumping the spec version label Nov 11, 2024
@sam0x17
Copy link
Contributor

sam0x17 commented Nov 11, 2024

@eagr
Copy link
Contributor Author

eagr commented Nov 12, 2024

yeah as expected, kinda wanted to watch it fail first. I'll disable the lint on that line then it should pass

@eagr
Copy link
Contributor Author

eagr commented Nov 12, 2024

I think #[allow(unknown_lints)] is meant for situations like this (to prevent the compiler from failing when using custom lints) right?

@sam0x17
Copy link
Contributor

sam0x17 commented Nov 12, 2024

looks good to me except I would prefer in the tests you use quote! directly so we don't have to do any string parsing. Makes the tests a little bit faster. See the tests for the other lints I did for inspiration! @eagr

@eagr
Copy link
Contributor Author

eagr commented Nov 13, 2024

done

@sam0x17 sam0x17 merged commit d23c372 into opentensor:devnet-ready Nov 13, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-spec-version-bump PR does not contain changes that requires bumping the spec version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants