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

Pyproject #19

Merged
merged 12 commits into from
Aug 26, 2024
Merged

Pyproject #19

merged 12 commits into from
Aug 26, 2024

Conversation

sveinbjornt
Copy link
Member

@sveinbjornt sveinbjornt commented Aug 21, 2024

  • 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)

Copy link
Member

@vthorsteinsson vthorsteinsson left a 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
Copy link
Member

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)
Copy link
Member

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?

Copy link
Member

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?

@@ -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
Copy link
Member

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?

@sveinbjornt sveinbjornt merged commit 25ec942 into main Aug 26, 2024
14 checks passed
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