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

centralize lint configuration #14236

Closed
andrewbanchich opened this issue Jul 11, 2024 · 4 comments
Closed

centralize lint configuration #14236

andrewbanchich opened this issue Jul 11, 2024 · 4 comments
Labels
A-lints-table Area: [lints] table C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-team-input Status: Needs input from team on whether/how to proceed.

Comments

@andrewbanchich
Copy link

Problem

Description

It would be really great to be able to centralize lint configurations across packages. The current limitations mean that, if a team maintains 20 packages in different repos, they will need to manage each package separately.

Is this something we'd be interested in adding?

Proposed Solution

I'd think we could have a feature where we could add something like:

[lints.src]
git = "https://github.com/team-repo/lint-config.git"

to Cargo.toml.

Notes

No response

@andrewbanchich andrewbanchich added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Jul 11, 2024
@epage epage added the A-lints-table Area: [lints] table label Jul 11, 2024
@epage
Copy link
Contributor

epage commented Jul 11, 2024

For myself, I find this true of a lot of aspects of my project (github actions, tool settings, etc) and I keep a repo of all of that (https://github.com/epage/_rust). I periodically merge this into each of my repos (would be great to automate). For repos that weren't forked off of this, I used git merge --allow-unrelated-histories. The main limitation is propagating package-level information in a workspace.

If we were to do something like this, we can add the sub-table as we've reserved all tool names though it blocks that name from ever being used by a tool (like lints.workspace does).

We would need to deal with

  • Fully specifying the source. Likely this wouldn't just be git rev/tag/branch but finding a specific location within the git repo
  • Potentially locking the source with a way to update it
  • Defining the source format (just another manifest? a custom format)

Another odd aspect of this is it being present in dependencies. iirc we apply the lint table within a dependency but to apply this would have us pulling arbitrary locations, ones that might not exist anymore, which would also slow things down. We might be able to say "eh, cap-lints means we likely won't be affected" and just drop these lints.

@epage epage added S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Jul 11, 2024
@epage
Copy link
Contributor

epage commented Oct 29, 2024

Some prior art mentioned in crate-ci/typos#1129

It would be nice if there was an option in the config file to extend from another config file, much like ruff or pyright does.

Currently, for ruff, I can for example do something like this in my pyproject.toml (docs)

[tool.ruff]
extend = ".company-submodule/ruff.toml"
line-length = 150

@epage
Copy link
Contributor

epage commented Oct 29, 2024

While not for lint control (as its not in config), we have a similar extension model being considered for config: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#config-include

@ehuss
Copy link
Contributor

ehuss commented Oct 29, 2024

The cargo team discussed this today and decided to close this issue for now. In general we would prefer that if we were to add any sort of includes, it should be a generic mechanism for including in any position (not just lints). Additionally, we would not want those includes to be from a remote network location, but instead only from paths (due to the many complexities with network-based includes). This may look something like the ruff example above.

However, we are generally wary of the complexity that might entail with manifest parsing, and the interaction with workspace inheritance (and possibly nested workspaces). We're not ruling out the possibility of adding includes in the future, but probably isn't on the visible horizon for now.

Thanks for the suggestion, though! This is something we had quite a bit of discussion about when we first introduced lints, and I imagine it will be explored more in the future.

@ehuss ehuss closed this as not planned Won't fix, can't repro, duplicate, stale Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints-table Area: [lints] table C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-team-input Status: Needs input from team on whether/how to proceed.
Projects
None yet
Development

No branches or pull requests

3 participants