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

Remove raw_pointer_deriving lint #29882

Merged
merged 3 commits into from Nov 18, 2015
Merged

Remove raw_pointer_deriving lint #29882

merged 3 commits into from Nov 18, 2015

Conversation

devonhollowood
Copy link
Contributor

Implement #14615

@rust-highfive
Copy link
Collaborator

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Manishearth
Copy link
Member

@bors r+

Looks good to me, thanks!

@bors
Copy link
Contributor

bors commented Nov 17, 2015

📌 Commit 5e8225a has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Nov 17, 2015

⌛ Testing commit 5e8225a with merge 71bcaa1...

@bors
Copy link
Contributor

bors commented Nov 17, 2015

💔 Test failed - auto-mac-64-opt

@Manishearth
Copy link
Member

../src/liblibc/src/lib.rs:13:21: 13:39 error: unknown `allow` attribute: `raw_pointer_derive`, #[deny(unknown_lints)] on by default
../src/liblibc/src/lib.rs:13 #![allow(bad_style, raw_pointer_derive, overflowing_literals, improper_ctypes)]
                                                 ^~~~~~~~~~~~~~~~~~
error: aborting due to previous error

You'll have to open a PR to https://github.com/rust-lang-nursery/libc first to remove that, and then update the submodule. Sorry about that. Let me know if you need help (or want me to do it for you) 😄

@Manishearth
Copy link
Member

@alexcrichton Do we consider breakages in allow lints (e.g. unknown_lints in this case) to be a breaking change? Especially since cargo does that lint cap thing.

@devonhollowood
Copy link
Contributor Author

I submitted a pull request to libc.
If we do consider this a breaking change, maybe it makes more sense to go back and change the default lint level from Warn to Allow instead of the changes here (which to my understanding would be non-breaking). If we do decide to do that, would it be cleaner to do so from a new pull request?

@alexcrichton
Copy link
Member

Nah this isn't a breaking change because it's just tweaking lints, which aren't considered breaking changes (due to --cap-lints)

@Manishearth
Copy link
Member

Cool.

@devonhollowood It landed, so you need to now update the submodule to rust-lang/libc@8531cc1 and add it to your commit. If you need help:

cd src/liblibc
git fetch origin
git checkout 8531cc11e196b7ae9072ae7b384a930ff7b00dfb
cd ..
git add liblibc
git commit --amend

or something like that.

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 17, 2015

📌 Commit 0823ee6 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Nov 18, 2015

⌛ Testing commit 0823ee6 with merge 50b969d...

bors added a commit that referenced this pull request Nov 18, 2015
@brson
Copy link
Contributor

brson commented Dec 11, 2015

This caused breakage. I was under the impression that lints are never supposed to be removed because of this problem and simply turned to no-ops.

@Manishearth
Copy link
Member

I thought lint changes are fine because of lint capping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants