Skip to content

Conversation

@nedbat
Copy link
Member

@nedbat nedbat commented Jan 14, 2020

@jambonrose This makes it work for both, though it's not nice. I wondered about mocking the creation of the plugins to track which had been made during the test, but I didn't quickly see a way to do that.

Also, I don't know if we should support 4.x and 5.x (which would double the already bulky list of tox environments), or just move to 5?

@nedbat nedbat mentioned this pull request Jan 14, 2020
3 tasks
@jambonrose
Copy link
Collaborator

I would be in favor of moving to a v2 plugin that supports:

  • Python 3.6+
  • Coverage 5+
  • Django 2.2+

To ease migration for others, I would advocate ensuring that v1 works with all those versions, and then that we drop support of previous versions of Python/Coverage/Django when we bump the major version of the plugin.

@nedbat
Copy link
Member Author

nedbat commented Jan 14, 2020

@jambonrose sounds good to me. One reason I have heard for continuing to support Python 3.5: it's the version installed on a still-supported LTS of Ubuntu: 16.04. But I don't feel strongly about it.

@nedbat
Copy link
Member Author

nedbat commented Jan 14, 2020

Also: I should add a supported way to get this information: coveragepy/coveragepy#920

@browniebroke
Copy link
Contributor

I think the build should go green if you update the branch now.

@jambonrose
Copy link
Collaborator

@nedbat : That's a good reason to support Python 3.5 . Additionally, I see now that Django 2.2 is also Python 3.5 compatible.

@nedbat
Copy link
Member Author

nedbat commented Jan 16, 2020

Updated.

Copy link
Contributor

@browniebroke browniebroke left a comment

Choose a reason for hiding this comment

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

@nedbat
Copy link
Member Author

nedbat commented Jan 17, 2020

@jambonrose Is this good to go? Do I merge, or do you?

@Tecktron
Copy link

What's the time line on getting this merged?

@jambonrose
Copy link
Collaborator

I will look at this tomorrow. Thanks all for your patience.

Sent with GitHawk

@jambonrose
Copy link
Collaborator

jambonrose commented Jan 23, 2020

The test suit for this PR ran with Coverage 4.5.4 .

I think we should:

  1. remove the pre-v5 restriction from setup.py
  2. update the Read Me to remove the note about Coverage 5
  3. run the tests on the CI with Coverage 5

Normally, I'd advocate for updating Tox to run both Coverage 4 and 5, but given our conversation about deprecation and the existing test run on Coverage 4, that feels unnecessary.

@nedbat
Copy link
Member Author

nedbat commented Jan 23, 2020

I made those changes.

@jambonrose jambonrose merged commit 3b3bc7a into master Jan 23, 2020
@jambonrose jambonrose deleted the nedbat/cov5 branch January 23, 2020 11:46
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.

5 participants