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

Fix some clippy warnings #680

Merged
merged 1 commit into from Dec 26, 2018

Conversation

Projects
None yet
2 participants
@EPashkin
Copy link
Member

EPashkin commented Dec 25, 2018

also add clippy check in travis

@EPashkin EPashkin force-pushed the EPashkin:fix_clippy branch 2 times, most recently from 989bb3f to c65e5c2 Dec 25, 2018

@sdroege
Copy link
Member

sdroege left a comment

Thanks :)

@@ -141,7 +141,7 @@ impl Parameters {
}
}

#[cfg_attr(feature = "cargo-clippy", allow(useless_let_if_seq))]
#[cfg_attr(feature = "cargo-clippy", allow(clippy::useless_let_if_seq))]

This comment has been minimized.

@sdroege

sdroege Dec 25, 2018

Member

You don't need the feature anymore

This comment has been minimized.

@EPashkin

EPashkin Dec 25, 2018

Author Member

You right, it already on stable too.

@@ -65,12 +65,12 @@ impl Signature {
(false, None)
}

pub fn eq(&self, other: &Signature) -> bool {
pub fn equals(&self, other: &Signature) -> bool {

This comment has been minimized.

@sdroege

sdroege Dec 25, 2018

Member

Or implement Eq here?

This comment has been minimized.

@EPashkin

EPashkin Dec 25, 2018

Author Member

I prefer to leave it this way, since we also have property_eq

This comment has been minimized.

@EPashkin

EPashkin Dec 25, 2018

Author Member

Hm, if I remove unneeded pub clippy don't nags about old name

@EPashkin EPashkin force-pushed the EPashkin:fix_clippy branch from c65e5c2 to 8c3c89b Dec 25, 2018

@@ -6,7 +6,7 @@ pub fn begin(w: &mut Write) -> Result<()> {
let v = vec![
"",
"#![allow(non_camel_case_types, non_upper_case_globals, non_snake_case)]",
"#![cfg_attr(feature = \"cargo-clippy\", allow(approx_constant, type_complexity, unreadable_literal))]",
"#![cfg_attr(feature = \"cargo-clippy\", allow(clippy::approx_constant, clippy::type_complexity, clippy::unreadable_literal))]",

This comment has been minimized.

@EPashkin

EPashkin Dec 25, 2018

Author Member

This will be with feature check until we don't go up from rust 1.28.0

This comment has been minimized.

@sdroege

sdroege Dec 25, 2018

Member

We have to switch to 1.31 anyway already, @GuillaumeGomez has that on his list already. Major parts of the ecosystem already don't compile with pre-1.31 anymore, including things like cc.

For GStreamer I already switched to 1.31 because of that.

This comment has been minimized.

@EPashkin

EPashkin Dec 25, 2018

Author Member

Thanks for info, then I remove feature here too.

This comment has been minimized.

@sdroege

sdroege Dec 25, 2018

Member

ok :) Doesn't make me happy but what can we do

@EPashkin EPashkin force-pushed the EPashkin:fix_clippy branch from 8c3c89b to c9026ec Dec 25, 2018

@EPashkin EPashkin force-pushed the EPashkin:fix_clippy branch from c9026ec to 8d7c96b Dec 25, 2018

@EPashkin EPashkin merged commit 32d1716 into gtk-rs:master Dec 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@EPashkin EPashkin deleted the EPashkin:fix_clippy branch Dec 26, 2018

@EPashkin EPashkin referenced this pull request Jan 5, 2019

Merged

Regen #125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.