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

Housekeeping #27

Merged
merged 12 commits into from
May 7, 2024
Merged

Housekeeping #27

merged 12 commits into from
May 7, 2024

Conversation

ikalnytskyi
Copy link
Owner

(see commit messages for details)

Even though I'm still trying to wrap my lines at 80 characters, over the
years I realized that blindly following the 80 characters rather harms
than helps. That's why I want to bump the line-length limit to 100
characters.
Default Ruff configuration tries to be as close as possible to Flake8,
so that the former can be considered a drop-in replacement of the
latter. Ruff, however, comes with many batteries included, and there are
many useful rules out there. This patch enables most of them, leaving
our the one the code base is not ready for for future.
Some platforms, such as Windows, may not have password-store available
for them, and some systems, such as my home machine, doesn't have it
permanently installed. Instead of failing a test run when password-store
is not available, it's better to skip those tests.
The `tmpdir` fixture is deprecated because it returns the legacy path
type. It's not recommended to use `tmp_path` as it returns the path type
from the standard library.
This is my personal preference I wasn't sure how to deal with ever since
I migrated to Ruff. Apparently, Ruff provides configuration options to
achieve what I want.
This plugin claims to support 5 Python versions: 3.8 to 3.12, and 3
operating systems: Linux, macOS and Windows. By testing all possible
combinations, it requires running 15 test jobs in parallel.

Unfortunately, Windows & macOS runners aren't fast to be found, and
sometimes one has to wait a significant amount of time for a pull
request to become "green". This is wasteful for me, as a sole
contributor, and GitHub, as a company that provides compute resources
for free.

In practice there's almost no benefits of testing all python versions on
Windows and macOS platforms as long as we test them elsewhere (Linux).
In most cases, either the python version is not supported regardless of
the platform it runs on, or the platform is not supported regardless of
the python version you run on it.

This patch decreases the testing matrix size by running tests on Windows
and macOS runners just once, using the latest available python version.
@ikalnytskyi ikalnytskyi force-pushed the chore/housekeeping branch 3 times, most recently from 70dc8bf to 84285a7 Compare May 7, 2024 12:21
I have never been a fan of poetry, but since this project is already
using it, let's keep it this way. What we have to do though is to make
sure we use the up-to-date poetry configuration and best practices.

This patch moves dependency definitions out from tox.ini to
pyproject.toml (poetry groups). This should help us to maintain
dependencies in one place only.
There are 16 deprecation warnings generated by gh actions today. We
better bump used versions to make sure things won't get broken
unexpectedly.
The GitHub Actions CI/CD can show errors in pull request per line. This
is much better than reading stdout/stderr outputs. This patch adds a
pytest plugin that can generate gh actions reports, and opts Ruff into
github actions output when ran on CI.
@ikalnytskyi ikalnytskyi force-pushed the chore/housekeeping branch from 84285a7 to 4650c32 Compare May 7, 2024 12:23
The test we have today doesn't check retrieved secret except for its
length. Even though in most cases testing the length most likely will be
enough, it's not robust enough. This patch makes the test better by
creating a known secret in password-store, retrieving it and then
testing that the secret is expected.
Almost all of them are checked now, with few exceptions that tend to
return false positives.
Do not use shell to retrieve a secret from password-store because (a)
it's less secure, and (b) it's one extra executable invocation.
@ikalnytskyi ikalnytskyi merged commit 35a5ace into master May 7, 2024
8 checks passed
@ikalnytskyi ikalnytskyi deleted the chore/housekeeping branch May 7, 2024 15:12
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