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

Migrate to tox-dev and v4 support #74

Closed
gaborbernat opened this issue Aug 29, 2021 · 11 comments
Closed

Migrate to tox-dev and v4 support #74

gaborbernat opened this issue Aug 29, 2021 · 11 comments

Comments

@gaborbernat
Copy link

Hello, would you consider moving the project under the tox-dev umbrella? See documentation under https://tox.readthedocs.io/en/rewrite/plugins.html#adoption-of-a-plugin-under-tox-dev-github-organization

Furthermore, tox v4 is getting ready and we'd like to make sure this plugin is supported from day 1, we're collecting feature gaps for this under tox-dev/tox#1974. Would be great if you could join our development chat under https://discord.gg/tox so we can assist with this. If you do so please drop in a line in the #plugin chat with the name of the repository you maintain. Thanks!

@gaborbernat
Copy link
Author

@gaborbernat gaborbernat mentioned this issue Aug 30, 2021
10 tasks
@ymyzk
Copy link
Owner

ymyzk commented Sep 1, 2021

Hello, would you consider moving the project under the tox-dev umbrella? See documentation under https://tox.readthedocs.io/en/rewrite/plugins.html#adoption-of-a-plugin-under-tox-dev-github-organization

Thanks for proposing this! Let me have a look at the document and think about it.

Furthermore, tox v4 is getting ready and we'd like to make sure this plugin is supported from day 1, we're collecting feature gaps for this under tox-dev/tox#1974.

I think we can consider migration to tox-dev and tox v4 support separately but I'm 100% agree on supporting tox-gh-actions from the day 1 of tox v4. I didn't know tox v4 is getting ready, I'm glad to hear that 🙂

I'll prioritize tox v4 support and will have a look at your PoC soon (like this weekend).

Would be great if you could join our development chat under https://discord.gg/tox so we can assist with this. If you do so please drop in a line in the #plugin chat with the name of the repository you maintain. Thanks!

Yep, will do that.

@gaborbernat
Copy link
Author

My POC is passing CI and all that so would recommend we use that as a working base 😊 but is not entirely feature full, I'm happy to discuss the gaps and changes needed from tox core to get there. However, before I move past the POC I wanted your eyes on it.

@ymyzk
Copy link
Owner

ymyzk commented Sep 5, 2021

Thanks for the PoC :) I could get ideas on how to use tox v4's APIs, although the PoC includes many changes irrelevant to the tox v4 support itself and the PoC's logic to override the envlist is different from tox v3. Considering that I decided to create a new branch and implement tox v4 support incrementally. https://github.com/ymyzk/tox-gh-actions/tree/tox4

It's mostly working locally and I plan to push the update to this branch soon.

@gaborbernat
Copy link
Author

Thanks for the PoC :) I could get ideas on how to use tox v4's APIs, although the PoC includes many changes irrelevant to the tox v4 support itself and the PoC's logic to override the envlist is different from tox v3. Considering that I decided to create a new branch and implement tox v4 support incrementally. https://github.com/ymyzk/tox-gh-actions/tree/tox4

It's mostly working locally and I plan to push the update to this branch soon.

I strongly disagree. The way you had implemented it in v3 is just not supported in tox v4, and you'll want my poc for things to make sense and be supported.

@ymyzk
Copy link
Owner

ymyzk commented Sep 5, 2021

I strongly disagree. The way you had implemented it in v3 is just not supported in tox v4, and you'll want my poc for things to make sense and be supported.

Could you elaborate a bit more on which part is not supported and how you want to fix that?

I'm trying not to break the current behavior as much as possible to make it easier for users to migrate from tox v3. This is one of the highest priorities to me.

@gaborbernat
Copy link
Author

Could you please elaborate on irrelevant changes?

@ymyzk
Copy link
Owner

ymyzk commented Sep 5, 2021

Could you please elaborate on irrelevant changes?

Sure. This commit contains https://github.com/gaborbernat/tox-gh-actions/commit/a39cab4b4753cad523e4a1e5d965aeb441d23050 a lot of irrelevant changes for adding tox v4 support to this plugin. Do we really need to use your coding style for supporting tox v4?

@gaborbernat
Copy link
Author

I don't think I've added any irrelevant changes, so you'll have to clearly name each of these changes to have a conversation about it. I'm not sure which stylistic changes you're referring to, but each is inline with what core tox does. So following the same style should make easier for people to contribute to both. 🤷‍♂️

@ymyzk
Copy link
Owner

ymyzk commented Sep 5, 2021

If we want to make styling changes, I would prefer to do it in a separate issue. The styling change commit itself is large and has different issues (e.g. adding Sphinx related metadata shouldn't be necessary for this plugin). It's making it difficult for me to review the changes. If we want to really prepare tox v4 support soon, I feel we should focus on what we really need. I don't want to discuss further about the styling issue in this issue.

By the way, considering the experience in this ticket, I'm leaning towards not to migrate the project under tox-dev at this point.

@gaborbernat
Copy link
Author

Sure 👍 you're free to do as you wish, if you don't want to explain in detail you don't have to. Good luck! 🍀

I'll note here that I might add a few other changes and release my interpretation of what a GitHub actions tox plugin should be.

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

No branches or pull requests

2 participants