search

ionelmc / tox-wheel

A Tox plugin that builds and installs wheels instead of sdist. Note that this plugin is obsolte as tox 4.0 already has wheel support.
BSD 2-Clause "Simplified" License
23 stars 10 forks source link

Add support for PEP 517/518 #5

Closed botant closed 4 years ago

botant commented 4 years ago

Here is my attempt to add support for PEP 517/518. Tests can be improved, and documentation needs to be updated, but I wanted to know what you think before continuing. Thanks!

ionelmc commented 4 years ago

At first glance this looks good. How will the develop mode be handled in pep518 mode?

botant commented 4 years ago

I haven't thought about develop mode yet to be honest, mainly because I don't have much experience with it. How does it work today under bdist_wheel?

On Sun, 28 Jun 2020, 23:16 Ionel Cristian Mărieș, notifications@github.com wrote:

At first glance this looks good. How will the develop mode be handled in pep518 mode?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-650829412, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPDXARYY5M2GZEQUC6TRY66MFANCNFSM4OKV274Q .

ionelmc commented 4 years ago

I think for now we need to disallow that (iow: give clear error to use that pep518 mode can't be used with develop mode).

There's no support for it yet, see https://github.com/pypa/pip/pull/8212 for what's worked on.

botant commented 4 years ago

Just committed a fix and test for the patch function.

About develop mode, am I right in interpreting that we currently don't build the wheel at all? I'm a bit puzzled about the idea of building a wheel and editing it in one go, kind of compiling and editing at the same time. I might be short sighted, but I can't see how these two things would go together.

botant commented 4 years ago

On a different note, we should add Python 3.8 to tox.ini since it is supported by tox.

Also, would you be open to adding Black and pre-commit to the project? I could have a stab at that in another PR.

ionelmc commented 4 years ago

Ah ... I think I may have forgotten that develop mode already disables the plugin.

Black - not really a fan of it. Pre-commit yes, but I already have some of the configuration of it in cookiecutter-pylibrary - it's just a matter of applying it (I usually do that before a release). It will have the 3.8 config so I wouldn't waste time with those.

botant commented 4 years ago

Black is indeed an acquired taste :-)

About the PR: this seems to work, but I'm relatively new to the inner workings of tox. I still need to update changelog, documentation, etc. before it can be merged.

On Mon, 29 Jun 2020, 23:09 Ionel Cristian Mărieș, notifications@github.com wrote:

Ah ... I think I may have forgotten that develop mode already disables the plugin.

Black - not really a fan of it. Pre-commit yes, but I already have some of the configuration of it in cookiecutter-pylibrary - it's just a matter of applying it (I usually do that before a release). It will have the 3.8 config so I wouldn't waste time with those.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-651395699, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPGMK6GVR4XX7ZVDHXTRZEGI3ANCNFSM4OKV274Q .

ionelmc commented 4 years ago

Well, I mean, if black will get you contribute more we can add it ... there's not much code in this project so it doubt it would produce lots of uglyness :) But wait till I reapply that cookiecutter so we can avoid conflicts.

botant commented 4 years ago

I do think that Black often produces ugly code, but it is useful for keeping a consistent style.

I'm keen to spend more time contributing anyway, which feels great and is very good for learning :-)

On Tue, 30 Jun 2020, 10:32 Ionel Cristian Mărieș, notifications@github.com wrote:

Well, I mean, if black will get you contribute more we can add it ... there's not much code in this project so it doubt it would produce lots of uglyness :) But wait till I reapply that cookiecutter so we can avoid conflicts.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-651679240, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPCCTSFJEDMBOJJGOPDRZGWKZANCNFSM4OKV274Q .

ionelmc commented 4 years ago

@botant alright so tests seem to pass (not sure what was going on before, maybe travis was having a bad time). How can we get this moving forward?

If you want to have a go at the black config please pull this first: https://github.com/ionelmc/tox-wheel/tree/skel-update

botant commented 4 years ago

Up to you, really. If you want, I can configure pre-commit with black, isort, and things like that. Otherwise we can move on as is since the tests are passing.

I haven't changed the documentation yet, and I also don't know if I should add my name anywhere, like a CONTRIBUTORS file.

Either way, I should be able to push another commit this weekend to finish this. Just let me know what you think.

Thanks!!

On Fri, 17 Jul 2020, 15:43 Ionel Cristian Mărieș, notifications@github.com wrote:

@botant https://github.com/botant alright so tests seem to pass (not sure what was going on before, maybe travis was having a bad time). How can we get this moving forward?

If you want to have a go at the black config please pull this first: https://github.com/ionelmc/tox-wheel/tree/skel-update

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-660145519, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPFT6A5G77M5CEFY3YLR4BPPPANCNFSM4OKV274Q .

ionelmc commented 4 years ago

Well then I can push the skel changes here and you can add the docs/black, yes?

There's a changelog/authors - feel free to change them.

botant commented 4 years ago

How about this: you push the skel changes, and I'll modify the documentation, etc.

I'll add black, isort and pre-commit in another PR, to make the diffs independent.

How does that sound?

On Fri, 17 Jul 2020, 16:24 Ionel Cristian Mărieș, notifications@github.com wrote:

Well then I can push the skel changes here and you can add the docs/black, yes?

There's a changelog/authors - feel free to change them.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-660167991, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPBFMZXTXSJVA46O5RDR4BUJDANCNFSM4OKV274Q .

ionelmc commented 4 years ago

I don't really care about small PRs - feel free to do whatever you like.

ionelmc commented 4 years ago

@botant you want to add anything else? I'm in the mood for making a release.

botant commented 4 years ago

Sorry, didn't get to it last weekend. I'll modify the docs and commit by the end of the day today, if that is ok.

On Fri, 24 Jul 2020, 13:42 Ionel Cristian Mărieș, notifications@github.com wrote:

@botant https://github.com/botant you want to add anything else? I'm in the mood for making a release.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ionelmc/tox-wheel/pull/5#issuecomment-663519285, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMJFPESSRW4JFJX2SAQTLTR5F6SPANCNFSM4OKV274Q .

ionelmc commented 4 years ago

Can't wait :-) Lemme know when you pushed.

botant commented 4 years ago

Done. I've added myself to AUTHORS.rst and updated README.rst. I've also added a new version to CHANGELOG.rst as 0.X.Y because I don't know what the next version will be, so please fix that. I didn't add Black to pre-commit.

It's been a pleasure to collaborate, and I'm looking forward to updating all my work projects with the new version!

ionelmc commented 4 years ago

Thanks!