Skip to content

Conversation

klausi
Copy link
Contributor

@klausi klausi commented Dec 16, 2018

See #3521

@klausi
Copy link
Contributor Author

klausi commented Dec 16, 2018

I did not touch CHANGELOG.md so far, it looks like the links there are auto-generated?

@flip1995
Copy link
Member

Thanks for your contribution and welcome to Clippy! 🎉

The changelog will be adapted automatically if you run $ ./util/dev update_lints.

The lint also needs to be registered as renamed. See rustc-docs

Since until now we never registered a lint as renamed, I'm not sure where to do this exactly. Could you try to call the register_renamed() function in the obvious places (clippy_lints/lib.rs)? If that doesn't work I will take a closer look at this myself.

@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 16, 2018
@klausi klausi changed the title chore(moduel_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language chore(module_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language Dec 16, 2018
@klausi
Copy link
Contributor Author

klausi commented Dec 16, 2018

Thanks! Ran $ ./util/dev update_lints and the CHANGELOG should be good now.

Tried to add the register_renamed() call at the end after all lints are registered, but that fails with "invalid lint renaming of stutter to module_name_repeat". What is the correct name of the new module_name_repeat lint so that it is valid here? Maybe it is not in the store where I'm running the register_renamed() on? How can I get the store which has the new module_name_repeat lint?

@flip1995
Copy link
Member

Yeah I thought of recalling something, that the lints actually get added later in the compiler. I need to give this a closer look tomorrow. :)

@phansch
Copy link
Contributor

phansch commented Dec 17, 2018

We could also deprecate the old one with a deprecation message that mentions the new lint name, if register_renamed doesn't work properly.

@@ -1028,6 +1028,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
unwrap::PANICKING_UNWRAP,
unwrap::UNNECESSARY_UNWRAP,
]);

store.register_renamed("stutter", "module_name_repeat");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we need clippy::stutter here?

@oli-obk
Copy link
Contributor

oli-obk commented Dec 17, 2018

Just a bikeshed now:

I think we need to name it module_name_repetitions or repetition_in_module_names according to the naming rules for lints

@flip1995 flip1995 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 17, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Dec 17, 2018

r=me with everyone involved alright with the new name.

@flip1995
Copy link
Member

flip1995 commented Dec 18, 2018

Let's merge this, so #3558 can use the changes here for renaming. (@klausi I hope you're also fine with merging this?)

@bors r=oli-obk

@bors

This comment has been minimized.

bors added a commit that referenced this pull request Dec 18, 2018
chore(module_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language

See #3521
@bors

This comment has been minimized.

@bors

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Dec 18, 2018

📌 Commit d74288e has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Dec 18, 2018

⌛ Testing commit d74288e with merge 61de562...

bors added a commit that referenced this pull request Dec 18, 2018
chore(module_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language

See #3521
@bors
Copy link
Contributor

bors commented Dec 18, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing 61de562 to master...

@bors bors merged commit d74288e into rust-lang:master Dec 18, 2018
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