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

py36+: remove requires_ordered_markup #7838

Merged
merged 1 commit into from Oct 3, 2020
Merged

py36+: remove requires_ordered_markup #7838

merged 1 commit into from Oct 3, 2020

Conversation

asottile
Copy link
Member

@asottile asottile commented Oct 2, 2020

See #7808

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Awesome work @asottile, thanks a ton for this!

I suggest hold off merging a day or two to give other reviewers a chance to take a look.

Other than that: do you plan to run pyupgrade --py36-plus to change string.format to fstrings?

Particularly I'm -0 the changes to git blame are worth it (but of course it is refreshing to be able to use it in new code).

@asottile
Copy link
Member Author

asottile commented Oct 3, 2020

Awesome work @asottile, thanks a ton for this!

I suggest hold off merging a day or two to give other reviewers a chance to take a look.

Other than that: do you plan to run pyupgrade --py36-plus to change string.format to fstrings?

Particularly I'm -0 the changes to git blame are worth it (but of course it is refreshing to be able to use it in new code).

this is just the first round of changes, there's a whole set of other changes that conflicted / depended on these that I'd have to rebase

as these are the less controversial ones I'm just going to merge these now -- we can always revert later

I do want to run pyupgrade and black in py36+ mode (you can see what these look like in #7811 -- they touch way fewer lines than I expected) once the manual code changes are done

fstrings also offer some performance gains after all :) (there's other things as well that'll be upgraded beyond fstrings iirc) so I think we should do it anyway.

additionally for blame, there's always --ignore-rev and/or --ignore-revs-file which fixes git blame for code reformats nicely!

I also still have to work out how to piece up the work for # type: comments since there's a significant amount that fail after automatic conversion. that's going to be a whole project in its own :)

@nicoddemus
Copy link
Member

as these are the less controversial ones I'm just going to merge these now -- we can always revert later

I agree they are not controversial, it was more to have a second set on eyes on the PRs, but if this will impact other immediate then by all means go for it.

I do want to run pyupgrade and black in py36+ mode (you can see what these look like in #7811 -- they touch way fewer lines than I expected) once the manual code changes are done

Ahh OK. I would expect them to touch a whole lot, but if they are few then I agree it is fine. 👍

I also still have to work out how to piece up the work for # type: comments since there's a significant amount that fail after automatic conversion. that's going to be a whole project in its own :)

👍

@asottile asottile merged commit a23666d into pytest-dev:master Oct 3, 2020
@asottile asottile deleted the py36_requires_ordered_markup branch October 3, 2020 02:48
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.

None yet

2 participants