Skip to content

Conversation

bossmc
Copy link
Contributor

@bossmc bossmc commented Jul 27, 2022

Original change written by @flip1995 I've simply rebased to master and fixed up the formatting/tests. This change teaches the configuration parser which config key replaced a deprecated key and attempts to populate the latter from the former. If both keys are provided this fails with a duplicate key error (rather than attempting to guess which the user intended).

Currently this on affects cyclomatic-complexity-threshold -> cognitive-complexity-threshold but will also be used in #8974 to handle blacklisted-names -> disallowed-names.

changelog: deprecated configuration keys are still applied as if they were provided as their non-deprecated name.
  • cargo test passes locally
  • Run cargo dev fmt

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Jarcho (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 27, 2022
@flip1995
Copy link
Member

I only vaguely remember writing this 😅 Thanks for seeing it to an end!

@Jarcho
Copy link
Contributor

Jarcho commented Jul 27, 2022

One small issue. This won't warn about duplicate fields when the old name appears after the new name. It's not a big deal as you still get the warning about the deprecated field.

Otherwise LGTM.

@bossmc
Copy link
Contributor Author

bossmc commented Jul 28, 2022

I've added a check to handle duplicates in either order and added a test for duplicates. I also extended the error message to be clear what's happening if one of the duplicates is using the deprecated name (though this only works on one direction).

@Jarcho
Copy link
Contributor

Jarcho commented Jul 29, 2022

Thank you. That should be good enough. The deprecation warning mentions both names in either case.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2022

📌 Commit ea25ef1 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 29, 2022

⌛ Testing commit ea25ef1 with merge 53a09d4...

@bors
Copy link
Contributor

bors commented Jul 29, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 53a09d4 to master...

@bors bors merged commit 53a09d4 into rust-lang:master Jul 29, 2022
@bossmc bossmc deleted the use-deprecated-config branch September 13, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants