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

chore: small adjustments #894

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

tcoratger
Copy link
Contributor

@tcoratger tcoratger commented Nov 26, 2024

Description

@Pratyush Here I propose a slightly different formalism to manage lints compared to what we had before on the repo. This formalism inherits from what is already done on reth https://github.com/paradigmxyz/reth/blob/83af493179393c81dc2c48374306f2b88cc075db/Cargo.toml#L160-L254 with a curated list of lints that have already proven themselves.

In this methodology, lints are directly introduced into the cargo.toml file and then dispatched to all the crates inside the repo via

[lints]
workspace = true

So it's easier to manage and more uniform. For now I have only applied it to the poly crate but we can adopt this for the other crates in follow up PRs if you agree.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the GitHub PR explorer

@tcoratger tcoratger requested review from a team as code owners November 26, 2024 17:11
@tcoratger tcoratger requested review from z-tech, Pratyush and mmagician and removed request for a team November 26, 2024 17:11
Copy link
Member

@Pratyush Pratyush 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 PR, this is great! If you could propagate this to the other crates in the repo as well that would be lovely =). We should also add it to the template https://github.com/arkworks-rs/template so that future repos will inherit it

@tcoratger
Copy link
Contributor Author

Thanks for the PR, this is great! If you could propagate this to the other crates in the repo as well that would be lovely =). We should also add it to the template https://github.com/arkworks-rs/template so that future repos will inherit it

@Pratyush Yes no problem, will propagate this in follow up PRs, one for each crate so that we have an easier review each time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants