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 clippy warnings/errors #436

Merged
merged 4 commits into from
Oct 26, 2023
Merged

Fix clippy warnings/errors #436

merged 4 commits into from
Oct 26, 2023

Conversation

alancai98
Copy link
Member

@alancai98 alancai98 commented Oct 24, 2023

Seems like GH Actions CI has been failing since a recent clippy change turned a warning into an error. PR also fixes a different clippy warning.

This should fix the GitHub Actions failure seen in #435.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@alancai98 alancai98 self-assigned this Oct 24, 2023
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (fbaac0f) 81.92% compared to head (81a1918) 81.91%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #436      +/-   ##
==========================================
- Coverage   81.92%   81.91%   -0.02%     
==========================================
  Files          63       63              
  Lines       16728    16718      -10     
  Branches    16728    16718      -10     
==========================================
- Hits        13705    13694      -11     
+ Misses       2470     2460      -10     
- Partials      553      564      +11     
Files Coverage Δ
partiql-value/src/list.rs 75.00% <0.00%> (+6.14%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link

github-actions bot commented Oct 24, 2023

Conformance comparison report

Base (fbaac0f) ac36638 +/-
% Passing 90.35% 90.35% 0.00%
✅ Passing 5731 5731 0
❌ Failing 612 612 0
🔶 Ignored 0 0 0
Total Tests 6343 6343 0

Number passing in both: 5731

Number failing in both: 612

Number passing in Base (fbaac0f) but now fail: 0

Number failing in Base (fbaac0f) but now pass: 0

@alancai98 alancai98 changed the title [do not merge] Check if CI action works for non-fork branch Fix clippy warnings/errors Oct 24, 2023
Comment on lines -186 to -201
let mut l = self.0.iter();
let mut r = other.0.iter();

loop {
match (l.next(), r.next()) {
(None, None) => return Some(Ordering::Equal),
(Some(_), None) => return Some(Ordering::Greater),
(None, Some(_)) => return Some(Ordering::Less),
(Some(lv), Some(rv)) => match lv.partial_cmp(rv) {
None => return None,
Some(Ordering::Less) => return Some(Ordering::Less),
Some(Ordering::Greater) => return Some(Ordering::Greater),
Some(Ordering::Equal) => continue,
},
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Having both Ord and PartialOrd implemented (with a non-default implementation) results in a clippy error: https://rust-lang.github.io/rust-clippy/master/index.html#/incorrect_partial_ord_impl_on_ord_type

Copy link
Contributor

@am357 am357 left a comment

Choose a reason for hiding this comment

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

Nice dive deep! Thank you for the fix!

@alancai98 alancai98 merged commit 57ce9b8 into main Oct 26, 2023
18 of 19 checks passed
@alancai98 alancai98 deleted the test-gh-actions-ci branch October 26, 2023 20:17
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.

2 participants