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

Sorted CSV #12

Merged
merged 1 commit into from
Jul 7, 2024
Merged

Sorted CSV #12

merged 1 commit into from
Jul 7, 2024

Conversation

amazingrando
Copy link
Contributor

Using macOS' Numbers app, I sorted the CSV by Publisher, then Title.

To review this PR:

  • Confirm the changes in sorting

@amazingrando amazingrando mentioned this pull request Jul 2, 2024
2 tasks
@johnhelmuth
Copy link
Collaborator

I think we are going to have to agree on a sorting collation to use for the check (which is en_US.UTF-8) to get consistent results.

Tonight, I will see if it's possible to change the configuration for Mac OS Numbers to do this, or what it is using and change the Check CSV order check to match that... and add something to the README about how to sort it before commits using that collation order.

Alternatively, we could add a script to do the sorting as part of a "build process", where we also set the package.json version number (Issue #10), which would run as part of a Github Action, allow it to be sorted and the sort checked with the same configuration.

@amazingrando
Copy link
Contributor Author

I'm good with whatever you decide on for sorting standards.

@johnhelmuth
Copy link
Collaborator

Merging this branch, adding a "test" script to package.json in a different PR that will do the check order step in a system agnostic way, so that we do not have to make sure we are all using the same tools to re-order the list.

@johnhelmuth johnhelmuth self-requested a review July 7, 2024 01:56
Copy link
Collaborator

@johnhelmuth johnhelmuth left a comment

Choose a reason for hiding this comment

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

This ordering looks good to me if it's good to @amazingrando, and it works with the code to check ordering in the a new PR to come.

@johnhelmuth johnhelmuth merged commit 93d675b into main Jul 7, 2024
1 check failed
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