Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix incomplete sorting. #4795

Merged
merged 8 commits into from
Jan 27, 2022
Merged

Fix incomplete sorting. #4795

merged 8 commits into from
Jan 27, 2022

Conversation

eskimor
Copy link
Member

@eskimor eskimor commented Jan 27, 2022

No description provided.

@eskimor eskimor added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Jan 27, 2022
@eskimor eskimor requested a review from drahnr January 27, 2022 12:46
(Some(a_height), Some(b_height)) => {
let height_ord = a_height.cmp(&b_height);
if height_ord == Ordering::Equal {
a.candidate_hash.cmp(&b.candidate_hash)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jakoblell jakoblell added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Jan 27, 2022
if check_equal(previous, current) {
return true
}
previous = current;
Copy link
Member

Choose a reason for hiding this comment

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

Personally preferred the previous functional-style impl with windows(2). But this one is also readable, so no complaints.

Copy link
Contributor

@drahnr drahnr Jan 27, 2022

Choose a reason for hiding this comment

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

I actually prefer this one, it has no unreachable! or implicit unreachable, it's imho cleaner, yet a tad less idiomatic.

@ordian ordian mentioned this pull request Jan 27, 2022
@drahnr
Copy link
Contributor

drahnr commented Jan 27, 2022

bot merge

@paritytech-processbot paritytech-processbot bot merged commit f0041c4 into master Jan 27, 2022
@paritytech-processbot paritytech-processbot bot deleted the rk-fix-sorting branch January 27, 2022 18:16
ordian added a commit that referenced this pull request Jan 27, 2022
* master:
  Fix incomplete sorting. (#4795)
  Companion for better way to resolve `Phase::Emergency` via governance #10663 (#4757)
  Refactor and fix usage of `get_session_index()` and `get_session_info_by_index()` (#4735)
  `relay chain selection` and `dispute-coordinator` fixes and improvements (#4752)
  Fix tests (#4787)
  log concluded disputes (#4785)
  availability-distribution: look for leaf ancestors within the same session (#4596)
  Companion for Use proper bounded vector type for nominations #10601 (#4709)
  Fix release profile (#4778)
  [ci] remove publish-s3-release (#4779)
  Companion for substrate#10632 (#4689)
  [ci] pipeline chores (#4775)
  New changelog scripts (#4491)
@chevdor chevdor mentioned this pull request Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants