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

docs: add docstring to library root module #2494

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Jul 15, 2023

Description

This PR fixes #2493 by adding a module-level docstring to sopel/__init__.py. I just copied the first two sentences from https://sopel.chat

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • [N/A] No issues are reported by make qa (runs make quality and make test)
  • [N/A] I have tested the functionality of the things this change touches

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Jul 15, 2023

And actually, digging further into this, the issue is no longer actually present after Python 3.7, when PEP 538 was implemented to coerce the C locale to UTF-8. I believe we could remove the restriction to use only ASCII text in that module, and the associated warning might be superfluous now as well. On the other hand, the LC_ALL setting can still influence the default encoding used for files, stdio, and pipes, so maybe it would be wise to leave the warning about the legacy locale until PEP 686 makes that problem go away. We could drop the "Python 3 [does] stupid things" bit, though 😅

I'll leave that out of this PR unless y'all agree that they can go, but the default behavior since 3.7 is to coerce these problematic locales to UTF-8, so the problem just doesn't exist in versions we support now anyway.

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Let's remove the rant now. 😁

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Jul 16, 2023

Done. I preserved the warning about non-UTF-8 locale but softened the language of it a bit. I'm not really sure it's a concern anymore with the changes that happened since Python 3.7, but I'm not confident enough to remove it entirely for the handful of users who will have a locale that is not using UTF-8 and is not coerced to same.

@dgw dgw added this to the 8.0.0 milestone Jul 17, 2023
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Sure, why not. (I dismissed the CodeQL alert about running print() at the module level. This is "something's seriously screwed up in the environment" code, so I think we can leave it be.)

@dgw dgw merged commit 271b306 into sopel-irc:master Jul 19, 2023
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.

Root module description is a mini-rant about LC_ALL rather than a description of the library
3 participants