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

Use native bytes value as the default sort order#19025

Merged
zoracon merged 1 commit intoEFForg:masterfrom
cschanaj:native-bytes
Apr 14, 2020
Merged

Use native bytes value as the default sort order#19025
zoracon merged 1 commit intoEFForg:masterfrom
cschanaj:native-bytes

Conversation

@cschanaj
Copy link
Copy Markdown
Collaborator

As per the documentation sort(1)

***  WARNING  ***  The locale specified by the environment affects sort
       order.  Set LC_ALL=C to get the traditional sort order that uses native
       byte values.

@cschanaj cschanaj requested a review from zoracon March 16, 2020 21:10
@zoracon
Copy link
Copy Markdown
Contributor

zoracon commented Apr 13, 2020

Could you specify impact it has on the sort order. I get the localization aspect but was this causing some sort of error if the native bytes weren't in use?

Not questioning to be contrary, just genuinely trying to understand what the underlying issue was.

@pipboy96
Copy link
Copy Markdown
Contributor

@zoracon The main issue is that sorting may be inconsistent between machines that have different locales.

@cschanaj
Copy link
Copy Markdown
Collaborator Author

@zoracon It is like what @pipboy96 described, when different contributors use a different locale, the diff of the ruleset-whitelist.csv might become difficult to review. This PR will make the automated change much more reproducible.

@zoracon
Copy link
Copy Markdown
Contributor

zoracon commented Apr 14, 2020

Thank you both for explaining a bit further, I'm not as sharp as possible of seeing multiple cases for contributors around the world. I also like a running record of commentary for changes. So I appreciate it and will merge this in. Thank you!

@zoracon zoracon merged commit 0bbd025 into EFForg:master Apr 14, 2020
@cschanaj cschanaj deleted the native-bytes branch April 14, 2020 00:59
@Bisaloo Bisaloo mentioned this pull request May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants