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

Improve backwards compatibility in sorting #47489

Merged
merged 3 commits into from
Nov 8, 2022
Merged

Conversation

LilithHafner
Copy link
Member

Fixup for #45222

This is not technically required as that dispatch system is undocumented and therefore internal. Nevertheless, it has zero runtime cost and might make some folks' lives easier.

See: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/6b14101_vs_97ccb97/StatisticalGraphics.primary.log, for example

From @KristofferC's request on slack

@LilithHafner LilithHafner added the sorting Put things in order label Nov 8, 2022
@KristofferC
Copy link
Member

@nanosoldier runtests(["StatisticalGraphics", "ExtendableSparse", "DLMReader", "IncompleteLU", "CartesianJoin"], vs=":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - no new issues were detected. A full report can be found here.

@LilithHafner
Copy link
Member Author

@nanosoldier runtests(["StatisticalGraphics", "DLMReader", "CartesianJoin"], vs=":master")

@@ -474,6 +474,7 @@ Characteristics:
(vanishingly rare for non-malicious input)
"""
const QuickSort = PartialQuickSort(missing, missing)
const QuickSortAlg = PartialQuickSort{Missing, Missing} # Exists for backward compatibility
Copy link
Member

Choose a reason for hiding this comment

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

I remember trying this but I think there was something it didn't handle. Might be wrong though...

@maleadt
Copy link
Member

maleadt commented Nov 8, 2022

Why retry these packages? They are tests that started passing on this PR. PkgEval also already retries nowadays, so it's not strictly necessary anymore to do so manually.

@KristofferC
Copy link
Member

Why retry these packages?

I think because the content of the PR changed since the last run.

@maleadt
Copy link
Member

maleadt commented Nov 8, 2022

Oh ok, carry on then! I was waiting for PkgEval to go idle so that I can upgrade it, hence me looking at the logs 🙂

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - no new issues were detected. A full report can be found here.

@KristofferC KristofferC merged commit f7d4edc into master Nov 8, 2022
@KristofferC KristofferC deleted the LilithHafner-patch-4 branch November 8, 2022 19:54
LilithHafner added a commit that referenced this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sorting Put things in order
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants