Conversation
Rob-Johnson
force-pushed
the
u/robj/py312-upgrade
branch
from
992627e to
c53ce06
Compare
Rob-Johnson
changed the base branch from
u/robj/remove-pylint-astroid
to
u/robj/pipx-virtualenv
Rob-Johnson
force-pushed
the
u/robj/py312-upgrade
branch
2 times, most recently
from
fc9f6f6 to
2388dde
Compare
Rob-Johnson
force-pushed
the
u/robj/py312-upgrade
branch
from
2388dde to
e4d9722
Compare
Rob-Johnson
requested review from
EvanKrall,
cuza,
giuli007,
ilkinmammadzada,
jfongatyelp and
nemacysts
nemacysts
left a comment
nemacysts
left a comment
There was a problem hiding this comment.
what i've tried to do in the past for python version upgrades is
- one commit that bumps the version
- one-to-N commit(s) that bump pre-commit hooks and adds any changes
- {misc commit if needed}
so that linting/formatting don't obscure actual changes - but I took a look at the non-python files and this should be fine :)
that said: i do think we have some extra requirements in requirements-minimal.txt if i'm not mistaken
| "paasta_tools.cli.cmds.mark_for_deployment.validate_service_name", | ||
| autospec=True, | ||
| return_value=True, | ||
| with ( |
There was a problem hiding this comment.
whaddayaknow - a black formatting change that doesn't make things uglier/i like!
| # dh_strip/objcopy corrupts _gpgme.cpython-312 at build time; exclude to keep packaging working. | ||
| override_dh_strip: | ||
| dh_strip --exclude=_gpgme.cpython- |
| graphviz | ||
| grpcio | ||
| gunicorn | ||
| gunicorn >= 25.0.1 |
There was a problem hiding this comment.
ideally we wouldn't really have any bounds here (and treat this like our internal pyproject.toml where we only add bounds if absolutely necessary) since in both cases we have a lockfile
...but there's already a decent number of lower-bounds here so i guess this is fine
| graphviz==0.8.2 | ||
| grpcio==1.62.2 | ||
| gunicorn==19.8.1 | ||
| gunicorn==25.0.1 |
There was a problem hiding this comment.
somewhat scary-looking bump - but afaict the breaking changes from https://github.com/benoitc/gunicorn/releases shouldn't affect us
yeah, I'm always a bit conflicted, because I also try and stick to 'every commit should pass the linter/tests and be safe to roll back to'. so I guess I could have made a bunch of commits and then squashed them all, but 🤷 . also, the github UI doesn't really make it easy to review indiviudal commits, so I'm not sure how much better things get for you. |
|
also opened #4227 to try and waste less of your time reviewing requirements-*.txt @nemacysts! |
yea, it's not great for leaving comments (i usually end up leaving the per-commit view if i actually need/want to leave a comment :p) - but in the usual best-case scenario where there's no comments it can make it easier to skip through files that only have no-op linting changes |
bumped six, py, pygpgme, black, splunk-sdk, venusian, gunicorn dropped tomli and exceptiongroup (now part of stdlib) add an override to the dh_strip interaction with py-gpgme lots of reformatting
Rob-Johnson
force-pushed
the
u/robj/py312-upgrade
branch
from
04909f3 to
c7f1c82
Compare
4 participants
sorry for the big diff - this was roughly the smallest single-commit I could make