Skip to content

Fix some clippy lints - #455

Merged
kngwyu merged 1 commit into
PyO3:masterfrom
Alexander-N:clippy
Apr 24, 2019
Merged

Fix some clippy lints#455
kngwyu merged 1 commit into
PyO3:masterfrom
Alexander-N:clippy

Conversation

@Alexander-N

Copy link
Copy Markdown
Member

No description provided.

@codecov

codecovBot commented Apr 21, 2019

Copy link
Copy Markdown

Codecov Report

Merging #455 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #455 +/- ##
==========================================
+ Coverage 87.64% 87.66% +0.01% 
==========================================
Files 63 63 Lines 3352 3356 +4 ==========================================
+ Hits 2938 2942 +4 
Misses 414 414
Impacted FilesCoverage Δ
src/class/sequence.rs80.8% <100%> (ø)⬆️
src/types/sequence.rs94.28% <100%> (+0.22%)⬆️
src/types/list.rs97.14% <100%> (ø)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77f0974...70c8d4b. Read the comment docs.

@codecov

codecovBot commented Apr 21, 2019

Copy link
Copy Markdown

Codecov Report

Merging #455 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## master #455 +/- ##
==========================================
+ Coverage 87.66% 87.66% +<.01% 
==========================================
Files 63 63 Lines 3355 3356 +1 ==========================================
+ Hits 2941 2942 +1 
Misses 414 414
Impacted FilesCoverage Δ
src/class/sequence.rs80.8% <100%> (ø)⬆️
src/types/sequence.rs94.28% <100%> (+0.22%)⬆️
src/types/list.rs97.14% <100%> (ø)⬆️
src/type_object.rs88.4% <0%> (-0.12%)⬇️
src/ffi3/objimpl.rs0% <0%> (ø)⬆️
src/ffi3/floatobject.rs100% <0%> (ø)⬆️
src/ffi3/modsupport.rs100% <0%> (ø)⬆️
src/ffi3/pyerrors.rs40% <0%> (ø)⬆️
src/types/dict.rs93.33% <0%> (ø)⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7149a1f...6d87934. Read the comment docs.

Comment threadsrc/types/list.rs
py: Python<'p>,
elements: impl IntoIterator<Item = T, IntoIter = U>,
) -> &'p PyList
pub fn new<T, U>(py: Python<'_>, elements: impl IntoIterator<Item = T, IntoIter = U>) -> &PyList

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a new grammar?
Great.

@kngwyukngwyuApr 24, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh it was stabilized a year ago rust-lang/rust#49458.
Maybe I have to learn Rust once more 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The edition guide and the official rust blog (esp. the release notes) are really good resources for that

@kngwyukngwyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threadsrc/types/sequence.rs Outdated

#[inline]
pub fn is_empty(&self) -> PyResult<bool> {
let len = self.len()?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I prefer to self.len().map(|l| l == 0).

Comment threadsrc/types/sequence.rs Outdated
let empty_seq = empty_list.cast_as::<PySequence>(py).unwrap();
assert_eq!(empty_seq.is_empty().unwrap(), true);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a nit but I don't like this empty line.

@kngwyu

kngwyu commented Apr 22, 2019

Copy link
Copy Markdown
Member

Thanks, I left some comments but mostly looks good.

@Alexander-N

Copy link
Copy Markdown
MemberAuthor

Ok, done.

@kngwyu

Copy link
Copy Markdown
Member

Thanks again

@kngwyu
kngwyu merged commit 5cc6b55 into PyO3:masterApr 24, 2019
Sign up for freeto 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.

3 participants

@Alexander-N@kngwyu@konstin