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

Switch to hatchling & pip-tools #22

Merged
merged 5 commits into from Sep 27, 2022
Merged

Switch to hatchling & pip-tools #22

merged 5 commits into from Sep 27, 2022

Conversation

samuelcolvin
Copy link
Contributor

@samuelcolvin samuelcolvin commented Sep 26, 2022

poetry was annoying me, also it's lock file was out of date.

Changes here:

  • use hatchling for build
  • use pip-compile (from pip-tools) to lock linting and testing requirements
  • tweak pre-commit
  • use pre-commit for linting
  • check github ref before release
  • add python 3.11 classifier
  • remove CI caching - seems to reduce CI time from ~1m20 to <20seconds, also simplifies CI setup

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2022

Codecov Report

Merging #22 (8bb65fe) into main (832acf9) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #22   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           61        61           
  Branches         5         5           
=========================================
  Hits            61        61           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@adriangb
Copy link
Contributor

A lot of changes for 1 PR 😉

poetry was annoying me, also it's lock file was out of date
use pip-compile (from pip-tools) to lock linting and testing requirements

I don't really think locking requirements for libraries is necessarily a good idea. Especially with pip-tools which AFAIK doesn't support locking with version specifiers or multiple platforms. I do it when I'm working solo on a project (because I can lock it to my machine and it makes installing things slightly faster). And there's something to be said for having the project run on a distribution of versions in CI / dev's local setups since it might uncover issues with specific versions of the dependencies (for example the black/click fiasco). So I would just remove all of the lockfiles and complexity that comes with them.

@adriangb
Copy link
Contributor

Tangential but since we're discussion CI setups / packaging / releases: I've found my favorite release strategy is to require every PR to make a valid version bump and cut a release for every commit to main. So instead of the flow being merge to main -> make an MR to bump the version -> make a GH release -> published to PyPi the flow becomes make an MR (that is forced to bump the version) -> merge to main -> automatic release and publish to PyPi. I think of it as "continuous delivery" for libraries.

@samuelcolvin
Copy link
Contributor Author

A lot of changes for 1 PR 😉

Sorry about that, I thought it would be okay since none of the changes relate to the library code itself.

don't really think locking requirements for libraries is necessarily a good idea.

We're not locking requirements for the library, only testing and linting requirements.

I'm so bored of CI suddenly failing due to "black vs click" compatibility or "flake8 vs pycodestyle", that I've started doing this.

There's not actually much more complexity, just testing.in and linting.in.

@Zac-HD do you have an opinion on this?

I've found my favorite release strategy is to require every PR to make a valid version bump and cut a release for every commit to main.

Can do if you really want, sounds to me like it would produce too much noice and also make it hard to track down what code was in what release. But can if you want.

Again probably worth asking @Zac-HD?

@adriangb
Copy link
Contributor

I'm so bored of CI suddenly failing due to "black vs click" compatibility or "flake8 vs pycodestyle", that I've started doing this.

Yeah it's frustrating but I'd rather that surface in CI than for devs. But that's just me, I don't have as many high profile projects as you 😅. But not a biggie keeping the lockfiles is fine for me.

Can do if you really want, sounds to me like it would produce too much noice and also make it hard to track down what code was in what release. But can if you want.

I wasn't necessarily proposing it for this project, just general discussion. I'm not sure who it would produce too much noise for, I guess just folks that subscribed to GitHub release notifications. Tracking down which version of the code was running is actually easier because there is a 1:1 relationship between library version and git commit. It's also easier to do "last version that didn't break me / have a bug" sorta searches with git bisect.

# This file is autogenerated by pip-compile with python 3.10
# To update, run:
#
# pip-compile --output-file=requirements/linting.txt requirements/linting.in
Copy link
Contributor

Choose a reason for hiding this comment

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

@samuelcolvin JFYK I recommend using the --resolver backtracking feature of pip-compile because it's much more robust at solving complex dependency graphs (boto3 and tensorflow send their regards) and because it's the default resolve in pip now (so using it keeps the results of pip-compile more in line with what you'd get from pip install).

@Zac-HD
Copy link
Member

Zac-HD commented Sep 26, 2022

I'm strongly in favour of pinning all the CI dependencies. I also like the "autorelease ever PR" style - we do that for Hypothesis and it's great. No strong opinion on how to lock, though I personally use pip-compile everywhere because it's dead simple and has been long-term stable.

For smaller projects I usually just set them to 'try twine upload --skip-existing' on every PR; the result is that you release whenever you bump the version number but don't have to release every PR.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

LGTM on a quick skim.

If we're using Black and Isort, want to try shed? It's my zero-config combination of those, pyupgrade, and a few smaller things.

@samuelcolvin
Copy link
Contributor Author

upload --skip-existing sounds awesome. Let's do it in a separate PR, this one already too big. 👀

Same with shed.

I'll add --resolver backtracking here tomorrow.

@samuelcolvin
Copy link
Contributor Author

I'll add --resolver backtracking here tomorrow.

Done, ready AFAIK.

@adriangb adriangb merged commit cf619a3 into main Sep 27, 2022
@adriangb adriangb deleted the hatch-and-friends branch September 27, 2022 15:23
adriangb pushed a commit to adriangb/annotated-types that referenced this pull request May 31, 2023
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.

None yet

4 participants