-
Notifications
You must be signed in to change notification settings - Fork 2
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
Pyproject #19
Pyproject #19
Conversation
sveinbjornt
commented
Aug 21, 2024
•
edited
Loading
edited
- Migrated project metadata over to pyproject.toml (PEP 621)
- Migrated away from deprecated pkg_resources package
- Fixed PyPy builds (broken by changes in latest distutil :/)
- Bumped minimum Python version to 3.9 (3.8 will soon reach EOL)
…tly provide importlib with package name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor questions / suggestions
pyproject.toml
Outdated
line-length = 120 | ||
|
||
[tool.black] | ||
line-length = 120 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were using 88 (the standard Black setting)?
@@ -566,9 +556,11 @@ def lines(self) -> Iterator[str]: | |||
self._line = 0 | |||
try: | |||
if self._package_name: | |||
stream = resource_stream(self._package_name, self._fname) | |||
ref = importlib_resources.files("islenska").joinpath(self._fname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the if above be changed to if self._fname
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...or "islenska"
-> self._package_name
?
src/islenska/bin_build.py
Outdated
@@ -72,6 +73,10 @@ | |||
if MACOS: | |||
extra_link_args = ["-stdlib=libc++", "-mmacosx-version-min=10.9"] | |||
|
|||
# On some systems, the linker needs to be told to use the C++ compiler | |||
# due to changes in the default behaviour of distutils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include a reference URL here for further reading? Where is this coming from?