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

Deprecate Python 2 formatting support #2523

Merged
merged 2 commits into from
Oct 31, 2021
Merged

Conversation

ichard26
Copy link
Collaborator

@ichard26 ichard26 commented Oct 6, 2021

Description

It's 2021. See also #2251.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@ichard26 ichard26 added C: cleanup Refactoring and removing dust :) C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases labels Oct 6, 2021
@ichard26 ichard26 changed the title Deprecate Python 2 formatting support [DO-NOT-MERGE] Deprecate Python 2 formatting support Oct 6, 2021
@ichard26
Copy link
Collaborator Author

ichard26 commented Oct 6, 2021

Just tagging this as a "do not merge" as all of this is very much up for debate.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Let's do it.

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

You don’t own me. I can merge if I want to. 😂

In seriousness, I +1 … there is enough support with the current version for Python 2 syntax. It ain’t changing.

Copy link
Collaborator

@felix-hilden felix-hilden left a comment

Choose a reason for hiding this comment

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

Big +1 for this and the whole timeline 👍

docs/faq.md Outdated Show resolved Hide resolved
# TODO: fully drop support and this code by December 1st, 2021.
if TargetVersion.PY27 in mode.target_versions or versions == {TargetVersion.PY27}:
err(
"DEPRECATION: Python 2 support will be removed after December 1st, 2021",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"DEPRECATION: Python 2 support will be removed after December 1st, 2021",
"DEPRECATION: Python 2 support will be removed by December 1st, 2021",

The warning doesn't match the comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something like" versions after black 22b0 will no longer support formatting python 2 code, pin to <21 for Python 2 support"

@JelleZijlstra
Copy link
Collaborator

In #2529 I suggest dropping support together with the other process changes proposed there, in January of 2022.

@ichard26 ichard26 requested review from ambv and zsol October 7, 2021 15:32
@ichard26 ichard26 force-pushed the deprecate-python-two branch 2 times, most recently from 947142d to 04b2b26 Compare October 21, 2021 22:10
@ichard26
Copy link
Collaborator Author

The current structure of the commits require that this is merged via a rebase so the split ends up being useful :)

Prepare for Python 2 depreciation

  • Use BlackRunner and .stdout in command line test

So the next commit won't break this test. This is in its own commit so we can just revert the depreciation commit when dropping Python 2 support completely.

@ichard26 ichard26 changed the title [DO-NOT-MERGE] Deprecate Python 2 formatting support Deprecate Python 2 formatting support Oct 22, 2021
@ichard26
Copy link
Collaborator Author

*Gentle nudge*

@zsol
Copy link
Collaborator

zsol commented Oct 27, 2021

lgtm.

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

LGTM - Lets merge?

Copy link
Contributor

@graingert graingert left a comment

Choose a reason for hiding this comment

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

The comments aren't consistent

@ichard26 ichard26 force-pushed the deprecate-python-two branch 2 times, most recently from 01cbded to 07ff8f9 Compare October 30, 2021 19:30
@ichard26
Copy link
Collaborator Author

One potential problem I do want to bring up is that right now each call to format_str with Python 2 warns ... that may be too noisy.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Noticed one typo.

I'm also still thinking about the feature detection stuff here and brought it up on Discord.

docs/faq.md Outdated Show resolved Hide resolved
@JelleZijlstra
Copy link
Collaborator

Printing for each file might be noisy, but properly doing it only once might require communication across asyncio workers, which I don't really want to have to deal with.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Let's merge it.

- Use BlackRunner and .stdout in command line test

So the next commit won't break this test. This is in its own commit so
we can just revert the depreciation commit when dropping Python 2
support completely.
@JelleZijlstra JelleZijlstra merged commit b21c0c3 into main Oct 31, 2021
@JelleZijlstra JelleZijlstra deleted the deprecate-python-two branch October 31, 2021 23:46
JelleZijlstra pushed a commit that referenced this pull request Nov 16, 2021
* Prepare for Python 2 depreciation

- Use BlackRunner and .stdout in command line test

So the next commit won't break this test. This is in its own commit so
we can just revert the depreciation commit when dropping Python 2
support completely.

* Deprecate Python 2 formatting support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: cleanup Refactoring and removing dust :) C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants