cachecontrol icon indicating copy to clipboard operation
cachecontrol copied to clipboard

Freshen up support tooling.

Open Flameeyes opened this issue 5 years ago • 4 comments

This includes a couple of fixes for 308 redirects, some extra debug logging, and a whole lot of tooling changes to use isort, black and pre-commit.

Flameeyes avatar Apr 13 '20 22:04 Flameeyes

So to come back from what I said on Twitter:

  • this is the set of changes that is safe and does not cover the whole half-rewrite I did, for that I want to mull it over a little more and come back with reviewable changes;
  • pip uses very few interfaces of cachecontrol, and that's good, it means that compatibility is not hard to maintain even while removing some of the features I removed to reduce complexity;
  • pip does have a modified FileCache implementation that might make sense to backport here, I'll take a look license compatibility later;
  • Python 2.7 support will be needed for pip compatibility, so I'll try to restore that in my branch as well; will probably use six to make it easier to convert this back to Python 3 only if possible.

What is your opinion on isort, black, mypy and pre-commit ? I can probably add all of those in on this pull request.

Flameeyes avatar Apr 17 '20 12:04 Flameeyes

I think these changes are looking good to me. It would be nice to break things up a bit. For example, adding all the SPDX, switching to six, etc. in different PRs. So far I didn't see anything concerning, but if there was an issue, at least we could chat about it independently.

I'm definitely a fan of black and started using it here a while back, that said, I don't think I automated the process and had been simply relying on my editor to do the needful. I'm also a fan of mypy, but again, just haven't had a chance to use it. Using isort also seems nice, although I'm a little concerned I could get annoyed with it if it doesn't work well all the time. Coming from Go, not having to reorder imports is definitely nice.

As for pre-commit, I'm a little more hesitant. I'm not a huge fan of magic automation and that feels a little magical to me. I'll be honest though and mention I'm saying this from ignorance so I'm happy to give it a try and learn something new!

In any case, if you want to submit PRs to use this stuff, let's do it individually so it is easier to understand and chat about.

Thanks!

ionrock avatar Apr 20 '20 15:04 ionrock

SG! Splitting up those that don't have obvious dependencies between each other.

The pre-commit "magic" is not really more magic than integrating with the editor — it basically just makes sure that black/isort are run before you complete the commit, but we can discuss that on the dedicated pull request.

I'll try to send them as independently as I can.

Flameeyes avatar Apr 20 '20 16:04 Flameeyes

Sent a few changes through already, I would prefer getting those merged before I rebase the black/isort parts because there'd be a lot of conflicts on both otherwise :(

Flameeyes avatar Apr 20 '20 17:04 Flameeyes