Changed NonCamelCaseTypes lint to warn by default #12290
Conversation
| @@ -260,6 +260,7 @@ mod imp { | |||
| pub static EPOLLHUP: libc::c_int = 0x010; | |||
| pub static EPOLLONESHOT: libc::c_int = 1 << 30; | |||
|
|
|||
| #[allow(non_camel_case_types)] | |||
There was a problem hiding this comment.
Maybe this attribute can go on mod imp along with the dead_code one?
There was a problem hiding this comment.
Sorry, I am not sure what you mean.
Could you please be a bit more precise?
There was a problem hiding this comment.
Sure.
You've attached allow(non_camel_case_types) to 4 types in this module (they're all grouped inside the mod imp on line 247), so I think it might be neater to just make this attribute apply to the whole module, like how the #[allow(dead_code)] one on line 246 applies to the whole module.
There was a problem hiding this comment.
This is quite a common pattern in libnative, so you may just want to move the allow to the crate level instead of the module level.
There was a problem hiding this comment.
Thanks @huonw, I get it now.
@alexcrichton I did it the way you suggested, however I am not sure if this is not counter productive to the main goal of this issue (promoting standards for new code). On the other hand, I am quite sure that anyone who will write new code for libnative will keep standards in mind.
|
This looks good to me, feel free to comment on the PR whenever you update it because sadly github doesn't sent out notifications on force-pushes. Also, could you squash the commits into just one? Thanks! |
|
@alexcrichton If it was possible, I would like to turn on the linters one by one and also send separate pull requests for each of them. Even just with this one, when I set it to warn there were more than 300 errors waiting for me. |
|
Sure thing, sounds good to me. Thanks for doing this work, it's much appreciated! |
|
@alexcrichton Sorry for the mess, new issues were introduced in master in the meantime. It should now pass the test suite. |
|
No worries, thanks! |
|
Sorry again @alexcrichton, for some reason I missed that timer. It should really be good to go now. Thanks for your patience. |
Added allow(non_camel_case_types) to librustc where necesary Tried to fix problems with non_camel_case_types outside rustc fixed failing tests Docs updated Moved #[allow(non_camel_case_types)] a level higher. markdown.rs reverted Fixed timer that was failing tests Fixed another timer
|
@alexcrichton I missed some once again. I am wondering how is this possible, |
This first part of my attempts to fix #11432. In this one I only set NonCamelCaseTypes to warn by default and tried to fix errors that were reported by `make check`. Please feel free to let me know if I missed something or didn't do it the right way. Thanks.
This first part of my attempts to fix #11432.
In this one I only set NonCamelCaseTypes to warn by default and tried to fix errors that were reported by
make check.Please feel free to let me know if I missed something or didn't do it the right way.
Thanks.