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

CI Overhaul #6

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

gandersen101
Copy link

@gandersen101 gandersen101 commented May 29, 2023

This PR should (hopefully) provide a modern CI/CD framework for bourbaki-introspection and serve as a model for other bourbaki libraries. It is heavily influnced by this Hypermodern Python but has some additional changes either due to necessity or opinion.

There are a lot of changes here but very few to the library internals. Basically my changes can be summarized as:

  • Moving the package management to poetry.
  • Introducing flake8 and mypy, the latter of which was not happy.
  • Introducing nox to automate our various lint/check/test steps over multiple Python versions.
  • Introducing pre-commit.
  • Migrating to GitHub Actions and adding some basic workflow logic there.

The package will test successfully on Python 3.7 and 3.8, but fails to run on 3.9. The package will fail the current flake8 and mypy checks for all Python versions checked. Feel free to modify their strictness/behavior as you see fit.

[flake8]
# B = bugbear
# B9 = bugbear opinions
# BLK = black style warnings
Copy link
Author

Choose a reason for hiding this comment

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

I'm having flake8 check black and isort formatting because I prefer to have flake8 statically check formatting rather than running Git modifying code in CI, although the difference is pretty arbitrary.

Copy link
Author

Choose a reason for hiding this comment

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

I haven't gotten a ton of use out of this personally but it's an easy add and I like it in theory. Described here.

- run: pip install nox poetry
- run: nox --python 3.8
- run: poetry build
- run: poetry publish --username=__token__ --password=${{ secrets.PYPI_TOKEN }}
Copy link
Author

Choose a reason for hiding this comment

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

You'll need to set up a PyPI API token in GitHub but this will allow you to release to PyPI with a Github release.

strategy:
matrix:
python-version: ['3.9', '3.8', '3.7']
os: [ubuntu-latest]
Copy link
Author

Choose a reason for hiding this comment

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

You may want to eventually add macos-latest here because you mention Mac support in your package classifiers. I'm omitting it for efficiency right now.

runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: ['3.9', '3.8', '3.7']
Copy link
Author

Choose a reason for hiding this comment

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

Not testing past 3.9 yet just for efficiency.

import nox
from nox.sessions import Session

nox.options.sessions = "lint", "mypy", "tests"
Copy link
Author

Choose a reason for hiding this comment

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

As mentioned in the .flake8 comment, I try to only have nox run non-Git-modifying code, so black and isort are left out of the default sessions. You can, however, still explicitly run them (e.x. locally).

Comment on lines +43 to +47
[tool.poetry.group.black.dependencies]
black = ">=22.3"

[tool.poetry.group.isort.dependencies]
isort = ">=5.10"
Copy link
Author

Choose a reason for hiding this comment

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

Taking advantage of poetry's ability to have an arbitrary number of dependency sections. The built package will only ever require what is specified in root dependencies section + optional extras.

Comment on lines +2 to +6
try:
from importlib.metadata import PackageNotFoundError # type: ignore
from importlib.metadata import version # type: ignore
except ImportError: # pragma: no cover
from importlib_metadata import version, PackageNotFoundError # type: ignore
Copy link
Author

Choose a reason for hiding this comment

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

Boilerplate __version__ code.

Comment on lines +26 to +33
try:
__version__ = version(__name__)
except PackageNotFoundError: # pragma: no cover
try:
__version__ = version(__name__.replace(".", "-"))
# Due to interaction between Poetry's name normalization and Python < 3.9
except PackageNotFoundError: # pragma: no cover
__version__ = "unknown"
Copy link
Author

Choose a reason for hiding this comment

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

More boilerplate __version__ code.

Copy link
Author

Choose a reason for hiding this comment

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

This had to change now that we're using an implicit namespace, but this test is probably superfluous as a whole, given that we're using a src organization that requires the package to be installed in order to test it.

@@ -0,0 +1,109 @@
[tool.poetry]
name = "bourbaki-introspection"
Copy link
Author

@gandersen101 gandersen101 Jun 2, 2023

Choose a reason for hiding this comment

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

I'm not 100% on how I feel about this, but poetry definitely wants you to only use the - separator in the package name, including namespaces.

@gandersen101 gandersen101 marked this pull request as ready for review June 2, 2023 03:09
@@ -0,0 +1,109 @@
[tool.poetry]
name = "bourbaki-introspection"
version = "0.5.10"
Copy link
Author

Choose a reason for hiding this comment

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

poetry lets you easily bump your version with poetry version.

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.

1 participant