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

3.8 test run fix #5448

Closed
wants to merge 2 commits into from
Closed

3.8 test run fix #5448

wants to merge 2 commits into from

Conversation

derlih
Copy link
Contributor

@derlih derlih commented Jan 28, 2021

What do these changes do?

Fix test run for python 3.6 on 3.8 branch

Are there changes in behavior for the user?

no

Related issue number

#5447

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

@derlih derlih requested a review from asvetlov as a code owner January 28, 2021 17:16
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jan 28, 2021
@derlih
Copy link
Contributor Author

derlih commented Jan 28, 2021

pypy run failed. It couldn't install typed_ast which is dependency of mypy and black.
Both of those packages are marked as implementation_name=="cpython" in `lint.txt'.
@webknjaz do you know why pip tries to install it?

@derlih derlih mentioned this pull request Feb 10, 2021
@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Feb 10, 2021

Looks like something to do with this change @greshilov:
e82eac8#diff-2676271ded3ba0170b6048b4fda0af7d5fcad17157ea8a1f217dfb505d8120a9R227

@webknjaz
Copy link
Member

@webknjaz do you know why pip tries to install it?

Because not all of the make targets changed pip install to use -r and -c: https://github.com/aio-libs/aiohttp/blob/9a09d1b/Makefile#L65 And now the file that used to have direct deps contains all the transitive deps too, without any env markers. That should be aligned with https://github.com/aio-libs/aiohttp/blob/9a09d1b/Makefile#L148.

@webknjaz
Copy link
Member

Can anybody take care of the Makefile before we merge this in?

@Dreamsorcerer
Copy link
Member

Can anybody take care of the Makefile before we merge this in?

#5470
I haven't got the slightest clue what I'm doing...

@webknjaz
Copy link
Member

Looks like #5470 fixed this in a more elegant manner.

@webknjaz webknjaz closed this Feb 25, 2021
@derlih
Copy link
Contributor Author

derlih commented Feb 28, 2021

@webknjaz
#5470 changes only dependency installation, but not type fixes for python 3.6

@Dreamsorcerer
Copy link
Member

I think he meant #5466, which seemed to be enough when I tried it. Once CI is back up, I'll double check and also deal with updating mypy to the latest release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants