Skip to content

Update sort-ini.py script #591

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

Merged
merged 3 commits into from
Mar 4, 2025
Merged

Update sort-ini.py script #591

merged 3 commits into from
Mar 4, 2025

Conversation

arilamstein
Copy link
Contributor

Update sort-ini.py to work with Python 3. Also run black on script.

Note that the README specifically instructs people to run this script. So updating it to work with Python 3 seems like a good idea.

As discussed in PR #590

@malemburg
Copy link
Member

Could you please undo the black run on the script so that it's easier to see what really changed ?

@malemburg
Copy link
Member

Also: I think it's better to open the file in text mode using 'utf-8' as encoding. That way you don't need the .encode() calls.

… of the changes are unnecessary. Revert changes made by black.
@arilamstein
Copy link
Contributor Author

Also: I think it's better to open the file in text mode using 'utf-8' as encoding. That way you don't need the .encode() calls.

This is a much more elegant solution than the one that I made (which was largely written by putting error messages into an LLM 😆 )

While there are no automated tests, I copied the state of config.ini from the checkin before mine (f9d4b85) and ran the version of sort-ini.py with the version of the code in this PR. The result is the same as the one in my PR that was accepted.

In short: the last 3 entries in the file are not in alphabetical order. Running the code in this PR puts them in alphabetical order (which is what I gather is the intention of the file).

Copy link
Member

@malemburg malemburg left a comment

Choose a reason for hiding this comment

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

LGTM

@malemburg malemburg merged commit 154f515 into python:main Mar 4, 2025
@malemburg
Copy link
Member

Thanks, @arilamstein

@hugovk
Copy link
Member

hugovk commented Mar 4, 2025

I've created PR #592 so we run sort-ini.py on the CI to ensure future changes remain sorted.

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.

3 participants