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

Add optional uvloop import #2258

Merged
merged 3 commits into from May 26, 2021
Merged

Add optional uvloop import #2258

merged 3 commits into from May 26, 2021

Conversation

cooperlees
Copy link
Collaborator

  • If we find uvloop in the env for black, blackd or black-primer lets try and use it
  • Add a uvloop extra install

Fixes #2257

Test:

  • Add ci job to install black[uvloop] and run a primer run with uvloop
    • Only with latest python (3.9)
    • Will be handy to compare runtimes as a very unoffical benchmark

- If we find `uvloop` in the env for black, blackd or black-primer lets try and use it
- Add a uvloop extra install

Fixes #2257

Test:
- Add ci job to install black[uvloop] and run a primer run with uvloop
  - Only with latest python (3.9)
  - Will be handy to compare runtimes as a very unoffical benchmark
@cooperlees
Copy link
Collaborator Author

Why are tests and lints skipped? Cause last commit only changed the CHANGES.md file?

@ichard26
Copy link
Collaborator

Yep. I made that change because of all of the documentation changes lately which don't need the (relatively) expensive primer and test workflows. In hidesight this isn't great UX even if it's technically speaking totally fine safety wise. We can revert it by simply removing the path-ignore keys in relevant workflow files.

Copy link
Collaborator

@zsol zsol left a comment

Choose a reason for hiding this comment

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

lgtm


jobs:
build:
# We want to run on external PRs, but not on our own internal PRs as they'll be run
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does internal vs external mean in this context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is copy pasta to maintain other CI behavior - this stops running the same CI twice for maintainers like me who make a branch and push to the black repo itself. @ichard26 did this and I like that it saves some resources. GitHub should really default to this to save CPU. One of us should go work there and claim the reward points.

@cooperlees cooperlees merged commit 754eecf into main May 26, 2021
@cooperlees cooperlees deleted the uvloop branch May 26, 2021 12:52
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.

Add optional extra uvloop install
3 participants