Conversation
f532691 to
4b71426
Compare
pyproject.toml
Outdated
| requires-python = ">= 3.7" | ||
| authors = [ | ||
| { name = "Philip Howard", email = "phil@pimoroni.com" }, | ||
| { name = "Garrett Berg" }, |
|
|
||
| dev-deps: | ||
| python3 -m pip install -r requirements-dev.txt | ||
| sudo apt install dos2unix |
There was a problem hiding this comment.
typically this kind of thing should NOT be in a makefile. Where is this even used?
There was a problem hiding this comment.
Say what? You can put whatever you want in a Makefile 🤣
There was a problem hiding this comment.
Or, to more specifically answer your question, where do you put this kind of thing?
This is used in the GitHub workflows to avoid encoding common patterns in GitHub's nonsense yml format. Plus it also makes most of the things you'd want to do during development a make x away.
The alternative (and a common CI pattern) is to use a shell script, but that has its own unique set of warts and is obnoxiously painful to invoke on a local machine (source ci/tools.sh && run_some_ci_action vs just make run_some_ci_action),
This is a pattern I use across all my libraries so I don't have to know or care how to invoke a bunch of arcane commands. Though, granted, sudo apt install dos2unix is very Debian-linux specific which is great for GitHub actions but not so great for hypothetical macOS using developers aaannnd I'm totally not writing this from a mac now 👀
There was a problem hiding this comment.
I would typically put it in you developer setup documentation. I won't block the CL on this
| { name = "Garrett Berg", email = "vitiral@gmail.com" }, | ||
| ] | ||
| maintainers = [ | ||
| { name = "Philip Howard", email = "phil@pimoroni.com" }, |
There was a problem hiding this comment.
I'm still reviewing CL's so I think I can be added here no?
There was a problem hiding this comment.
Will make the change, it’s pulled over from the boilerplate so it’s more an oversight than a deliberate attempt to erase you 🤣
|
LGTM |
|
Is there any reason this can't be merged? The current 1.0.0 release is still having those permission issues |
|
I'm not maintaining this project, and it seems the volunteer maintainer is not as focused any longer. If someone would like to maintain this project, I will be welcome to mentoring them. |
|
I would hazard a guess this has succumb to some bitrot in the interim. I'll give it a kick. |
Avoid duplication of requirements.
|
Thanks for the renewed effort guys! |
This set of changes brings gpio up to the latest standard (or at least the boilerplate I work with) of Python packaging, and includes conveniences like code spellcheck, import sorting and ruff linting in order to maintain code quality and catch bugs early.
TODO
test in Python 2.7+ for regressionsNotes
It looks like Python 2.7 support is probably not feasible against newer packaging, we should probably add the last couple of bug fixes in #26 to the 2.7-compatible 1.x.x branch.
sysfs gpio has been obsolete since 2016 and removed from kernels since 2020, with it limping on in forks of Linux mainline - https://lore.kernel.org/all/1445502750-22672-7-git-send-email-linus.walleij@linaro.org/
It's likely to be dropped from Raspberry Pi OS, too, so significant efforts to maintain this library will probably be redundant.
It's sensible to fixup the 1.0.0 branch for Python 2.7 and then update the packaging and release a 2.0.0 branch for Python 3.7+, with the implication that the 1.0.0 branch is a critical bugfix only legacy branch.
I appreciate legacy Python versions and sysfs gpio probably go hand-in-hand on more esoteric embedded devices, which is why I don't want to break the Python 2.7 library.