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

Changed numeric sorting to lexically resolving issue #851 #852

Merged
merged 2 commits into from
Nov 27, 2020

Conversation

matsduf
Copy link
Contributor

@matsduf matsduf commented Nov 20, 2020

The elements in the list (array) in the sort expression are not guaranteed to be numerical. Changed to lexical sorting. Resolves issue #851.

@matsduf matsduf added the A-TestCase Area: Test case specification or implementation of test case label Nov 20, 2020
@matsduf matsduf added this to the v2020.2 milestone Nov 20, 2020
@mattias-p
Copy link
Member

I agree with removing numeric sorting where you did, but it should also be added where @v4asnsets and @v6asnsets are built up. I wanted to make a review comment at the relevant line but Github wouldn't let me.

@matsduf
Copy link
Contributor Author

matsduf commented Nov 25, 2020

I do not understand what you want to have added. Please copy the lines of code and comment how you want it to be.

@matsduf
Copy link
Contributor Author

matsduf commented Nov 25, 2020

@mattias-p, do you want line 340 to change from

                push @v4asnsets, join( q{,}, sort @{$asnref} );

to

                push @v4asnsets, join( q{,}, sort { $a <=> $b } @{$asnref} );

and the equivalent for @v6asnsets?

@mattias-p
Copy link
Member

@matsduf Yes, exactly.

@matsduf
Copy link
Contributor Author

matsduf commented Nov 26, 2020

@matsduf Yes, exactly.

Updated with that.

@matsduf matsduf merged commit 34d2b53 into zonemaster:develop Nov 27, 2020
@matsduf matsduf deleted the corrected-sorting-Connectivity03 branch November 27, 2020 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-TestCase Area: Test case specification or implementation of test case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants