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

Tox #1090

Closed
wants to merge 23 commits into from
Closed

Tox #1090

wants to merge 23 commits into from

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Jun 22, 2021

No description provided.

@euri10 euri10 marked this pull request as ready for review June 22, 2021 13:23
@tomchristie
Copy link
Member

Is there a discussion on the motivation here?

@euri10
Copy link
Member Author

euri10 commented Jun 23, 2021

not really @tomchristie it's mainly a draft for me to play with it and see how the experience is vs the current tooling.

main points I'm thinking of are:

  1. current scripts test the source vs installed packages with tox, since we have many os / python versions this is I think a valuable tool that can avoid mistakes (one thing that comes to mind is the env marker after uvloop).
  2. pytest will deprecate usage of setup.cfg and don't recommand its usage https://docs.pytest.org/en/6.2.x/customize.html
  3. we can have almost (not flake8) the tooling config in one spot, ie pyproject.toml, (pytest, coverage, mypy, isort, black) and with one language (toml), and dont have to adapt to cfg parsing or ini files.
  4. dev experience is better inho when dealing with python versions discrepancies : currently if my dev uvicorn venv is python 3.9.5 and have something going on in 3.6 (which I'll discover only after the CI failed on 3.6) I have to locally:
rm -rf venv
asdf local python 3.6.13
./scripts/install
./scripts/test

while with tox I do tox -e py36

@euri10
Copy link
Member Author

euri10 commented Jun 23, 2021

another little point that makes things easier : we type things incrementally in uvicorn
when you reveal_type(somehting) the ./scripts/check will crash because flake8 is before the mypy checks.
with seperated tox venv you do a tox -e mypy and you're set,

so there are little things like that

@Kludex
Copy link
Sponsor Member

Kludex commented Jun 26, 2021

Shouldn't the CI use tox as well?

@Kludex
Copy link
Sponsor Member

Kludex commented Jun 26, 2021

I'm also in favor of it. But I think on the CI we should aim a style more similar to what asgiref does: https://github.com/django/asgiref/blob/main/.github/workflows/tests.yml

And maybe caching .tox similarly to #1095? (Maybe on another PR?)

@tomchristie
Copy link
Member

Personally I think we ought to have an excessively high bar for changes where we diverge the tooling approach used by differing encode projects.

If there's a discussion to have here I'd probably start it off by picking on single item of "I'd like to do this, but the tooling currently does that" and then dig into that single issue.

Doing things this way around feels a bit more tech-led "We should use tox" rather than solution-led "We'd like X to be different, because Y". (I know I might appear a bit picky here, but if encode as an org makes sense, then it probably makes sense to try to keep things in sync where possible, which means a super high bar to proposed tooling changes.)

@euri10 euri10 closed this Jun 28, 2021
@euri10 euri10 deleted the tox branch June 28, 2021 12:45
@euri10
Copy link
Member Author

euri10 commented Jun 30, 2021

points 1 2 and 4 above are indeed solution-led and this solves them, should we deal one by one ?

@tomchristie
Copy link
Member

Yes I think so.

Perhaps only dealing with one at a time, rather than starting separate discussions for each in parallel.
I don't have the bandwidth to properly look at them all together.

No 4 feels like a nicely isolated one, that I'd be keen on chatting about first.
Perhaps we could start a "discussion" on that, first?

@euri10
Copy link
Member Author

euri10 commented Jun 30, 2021

Yes I think so.

Perhaps only dealing with one at a time, rather than starting separate discussions for each in parallel.
I don't have the bandwidth to properly look at them all together.

No 4 feels like a nicely isolated one, that I'd be keen on chatting about first.
Perhaps we could start a "discussion" on that, first?

done !
encode/httpx#1728

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.

3 participants