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

Planned removal of license_template_path #5103

Closed
calebcartwright opened this issue Nov 22, 2021 · 9 comments · Fixed by #5357
Closed

Planned removal of license_template_path #5103

calebcartwright opened this issue Nov 22, 2021 · 9 comments · Fixed by #5357
Assignees

Comments

@calebcartwright
Copy link
Member

The purpose of this issue is to both communicate our intent to drop the unstable option license_template_path, as well as provide a forum for questions to be asked and/or concerns to be raised.

We do not yet have an exact deprecation date planned, but it will most likely occur in early 2022.

While we understand the utility this option provides, we've decided that rustfmt simply is not the right place/tool to provide this capability, and it's not an option that makes sense for us to try to stabilize and maintain/support long term.

If anyone is aware of any other community maintained tools/crates for checking/ensuring that Rust files have a license header then please feel free to post here for broader awareness.

@LoganDark
Copy link

The license header is part of the formatting of a file. I don't see how this is out of rustfmt's scope. Using rustfmt to ensure that all the license headers are present and up-to-date doesn't sound like such a bad idea.

@calebcartwright
Copy link
Member Author

Thank you for sharing your thoughts @LoganDark. However, I don't feel like you rebutted the substance of our expressed rationale, and I'd also request that folks refrain from snark in comments to ensure a constructive dialog.

The question is not whether header/license/copyright features are useful. They are.
The question is not whether rustfmt could provide such features. It could.
The question is not whether some users would find it convenient for rustfmt to provide such features. Some do.

The question is whether rustfmt should provide license/header features, whether rustfmt is the right tool to provide those capabilities for the ecosystem. We feel very strongly it is not, and that's why this unstable feature is going to be removed from rustfmt. Over the years, a number of rather auxiliary features were experimentally added to rustfmt simply because they "could be" which is not a sufficient rationalization in and of itself.

rustfmt's scope is, and always has been, explicitly to pretty print a parsed program's AST according to the rules of the Rust Style Guide. That means our actual scope is pretty printing Rust syntax. Full stop. We do of course have some additional features, but the farther away a feature is from that core mission, the farther outside our scope it is. This unequivocally includes checking license headers in files, which is quite far outside our scope; too far outside.

Additionally, consider that some portion of Rust users do not use rustfmt, either due to general dislike of code formatters or due to preferring their personal style. Ostensibly, this would include you as well based on this comment you left on a different issue. Having the license/header capabilities baked into rustfmt makes them unavailable to such users due to that strict, and unnecessarily coupling. Removing this semi-broken feature from rustfmt should help give room for a separate, dedicated tool to be added to the broader ecosystem in a way that's accessible to everyone regardless of whether they want to use rustfmt to format their Rust code.

Furthermore, while we do not base our decisions off those made in other formatters, I do think it's worth noting that the license/header features are not exactly ubiquitous in formatters. Rubocop (Ruby) is the only formatter I know offhand that does have this feature, and while I admit not being up to date on the latest with all these, to the best of my knowledge neither gofmt (Go), clang-format (C++/C/etc.), black (Python), autopep8 (Python), google-java-format (Java), nor prettier (JavaScript/TypeScript) provide this feature either. I also know that in recent times the license/header feature was requested and rejected as out of scope for both black and prettier.

@calebcartwright
Copy link
Member Author

Not cargo/crates available tooling, but sharing these options anyway in case they end up being useful for anyone:

https://github.com/google/addlicense
https://github.com/cloudposse/copyright-header

@LoganDark
Copy link

LoganDark commented May 20, 2022

I'd also request that folks refrain from snark in comments to ensure a constructive dialog.

I didn't intend for my comment to be snarky. I saw this in rustfmt's issues (as a matter of fact, while I was searching for the issue that you linked and evaluating whether rustfmt would be a fit for my project) and didn't understand why this was out of scope or an unreasonable idea.

Your comment would have corrected my misunderstanding just as well without the accusation.

As for this:

Having the license/header capabilities baked into rustfmt makes them unavailable to such users

The presence of an optional feature doesn't seem like it would alienate users, it's more likely that the opinionated lack of an option (like that issue you linked that I commented on) is what will prevent someone from using rustfmt. Your comments about scope are correct; I just find this particular point confusing. (FWIW, I'm not saying that the lack of this particular feature would make someone pass up rustfmt.)

@calebcartwright
Copy link
Member Author

I didn't intend for my comment to be snarky.
Your comment would have corrected my misunderstanding just as well without the accusation.

No worries, though worth noting that impact tends to matter more than intent, and you could've simply asked for more information instead of phrasing things the way you did with the bolding in rustfmt

I just find this particular point confusing

My point is that if rustfmt is the only tool that provides license checking features, that means that in order for users to use the license checking features they have to run rustfmt which will also format their code. That means users can't just use license checking features in isolation, they either have to use both or none, which is why it's a poor fit.

@LoganDark
Copy link

in order for users to use the license checking features they have to run rustfmt which will also format their code. That means users can't just use license checking features in isolation, they either have to use both or none, which is why it's a poor fit.

Thanks! This helps a lot.

@calebcartwright
Copy link
Member Author

Sure thing! I'm hopeful someone(s) will pull together and put a crate out on crates.io that does license checking well and does it right (at least better than our current buggy implementation), both because it's going to be needed sooner or later and because I think it would be a fun project.

If/when someone does develop or discover one please do let us know as we'll be happy to point to it from our docs/discussions.

@Garandor
Copy link

One more possible alternative is this python tool claiming to support rust sources:
https://github.com/johann-petrak/licenseheaders

@LoganDark
Copy link

One more possible alternative is this python tool claiming to support rust sources: https://github.com/johann-petrak/licenseheaders

Not really an alternative in the traditional sense, since it requires Python.

simonsso added a commit to NodleCode/chain that referenced this issue Jul 20, 2022
The feature license_template_path was depricated and has
been removed from rustfmt

rust-lang/rustfmt#5103
simonsso added a commit to NodleCode/chain that referenced this issue Jul 20, 2022
The feature license_template_path was deprecated and has been removed from rustfmt

rust-lang/rustfmt#5103
wischli added a commit to KILTprotocol/kilt-node that referenced this issue Aug 5, 2022
wischli added a commit to KILTprotocol/kilt-node that referenced this issue Aug 10, 2022
* feat: upgrade to Polkadot v0.9.27

* chore: bump nightly, sr-tool, base docker img

* fix: clippy clone runtime

* fix: clippy

* fix: remove deprecated license fmt checker

rust-lang/rustfmt#5103

* fix: clippy 2

* fix: clone

* chore: bump sub deps

* fix: clippy

* chore: bump serde

* ci: fix bump srtool to 1.62.0

* fix: remove kilt-support dep from clone runtime

* ci: bump parity base img
wischli added a commit to KILTprotocol/kilt-node that referenced this issue Aug 30, 2022
* feat: upgrade to Polkadot v0.9.27

* chore: bump nightly, sr-tool, base docker img

* fix: clippy clone runtime

* fix: clippy

* fix: remove deprecated license fmt checker

rust-lang/rustfmt#5103

* fix: clippy 2

* fix: clone

* chore: bump sub deps

* fix: clippy

* chore: bump serde

* ci: fix bump srtool to 1.62.0

* fix: remove kilt-support dep from clone runtime

* ci: bump parity base img
@calebcartwright calebcartwright unpinned this issue Dec 20, 2022
webguru9178 pushed a commit to webguru9178/kilt-node that referenced this issue Jan 8, 2024
* feat: upgrade to Polkadot v0.9.27

* chore: bump nightly, sr-tool, base docker img

* fix: clippy clone runtime

* fix: clippy

* fix: remove deprecated license fmt checker

rust-lang/rustfmt#5103

* fix: clippy 2

* fix: clone

* chore: bump sub deps

* fix: clippy

* chore: bump serde

* ci: fix bump srtool to 1.62.0

* fix: remove kilt-support dep from clone runtime

* ci: bump parity base img
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants