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

Trying out plone/meta #134

Merged
merged 24 commits into from
Mar 2, 2024
Merged

Conversation

reinout
Copy link
Owner

@reinout reinout commented Feb 2, 2024

@mauritsvanrees told me about plone/meta last weekend. Something like that is just what I need for my work projects :-)
So I'm testing it out with z3c.dependencychecker now.

There are two hacks that I need:

TODO: fix up the documentation if we're really going to use this.

Problem with plone/meta: the generated .editorconfig has a comment at the end of the line of two [...] sections. My emacs editorconfig-mode plugin doesn't like that. Probably something that needs fixing in the emacs plugin. Update: no, probably it needs fixing in meta. Fixed in plone/meta#208

One thing surprised me: I wanted to use triple single quotes to quote the extra_lines value as it contains double quotes. But plone/meta converts the .meta.toml back to triple double quotes.

@coveralls
Copy link

coveralls commented Feb 2, 2024

Coverage Status

coverage: 100.0% (+0.03%) from 99.967%
when pulling 0c5538c on config-with-default-template-5d3e918e
into 9673df6 on master.

@reinout
Copy link
Owner Author

reinout commented Feb 2, 2024

I see the coverage ends up at 0%, so there's probably still some old config lying around.
And I don't know if it is a good idea to include this project in plone/meta as it is no real plone product.
Anyway, @gforcada will probably enjoy the fact that I'm trying this out :-) Feedback/fixes/updates welcome.

@reinout reinout requested a review from gforcada February 2, 2024 15:52
@reinout
Copy link
Owner Author

reinout commented Feb 2, 2024

Oh, and tox -e circular complains about graphviz/cgraph.h not being found. Perhaps an OSX thingy in combination with python 3.12?

@gforcada
Copy link
Collaborator

gforcada commented Feb 3, 2024

Nice! 😊 🎉

Please, propose more options if you need them, as you say for the check-manifest configuration 👍🏾

The coverage report indeed, is not part of plone/meta but some coveralls integration that might be broken... not sure if all tests do pass then it might work again 🤷🏾

Finally, I see that circular does work now but dependencies fails with a stdlib_list, which, I think, is a conditional import (should be fixed if we merge at some point #117 ). For now, we can add it on the dependencies_ignores configure option ✨

@gforcada
Copy link
Collaborator

gforcada commented Feb 3, 2024

I see that the meta.yml related jobs coming from plone/meta fail due to missing some environment variables in GitHub (that should be fixed with plone/meta#211).

For the local test.yml failures, we should stop running flake8, isort and such, as that's part of the meta.yml provided ✨

@reinout I took the liberty to add a couple of commits on that direction, hopefully you ok with them, otherwise feel free to revert them 👍🏾

@gforcada gforcada force-pushed the config-with-default-template-5d3e918e branch from e5f05b8 to 3e1ba92 Compare February 3, 2024 17:13
@gforcada gforcada force-pushed the config-with-default-template-5d3e918e branch from b192d9c to b8c9e85 Compare February 3, 2024 17:20
@reinout
Copy link
Owner Author

reinout commented Feb 3, 2024

I wonder whether my .coveragerc is still needed with this setup?

@gforcada
Copy link
Collaborator

gforcada commented Feb 4, 2024

We can try to remove it and see what the coverage says 😄

At least, given that we would use tox, we can cleanup the list quite a bit I guess 🍀

@reinout
Copy link
Owner Author

reinout commented Feb 4, 2024

TODO (probably for myself) before I'm merging it: update the documentation ("tox -e xyz" instead of "venv/bin/pytest" and so).

We shouldn't put it into plone-meta's actual list of packages just yet because of the plone-test issue and the ignore-bad-ideas that I want to integrate a little bit better.

@gforcada
Copy link
Collaborator

gforcada commented Feb 5, 2024

The list you refer to is a for a feature of zopefoundation/meta that allows you to mass update repositories that are listed there.

plone/meta did not bother to remove that feature, though it is not so useful for us, as we are moving the tool a bit away from being project centric (i.e. only for zopefoundation related repositories), but more of a Plone/python generic tool.

@reinout
Copy link
Owner Author

reinout commented Feb 6, 2024

I think I've fixed all my issues in plone/meta#212

@reinout
Copy link
Owner Author

reinout commented Feb 6, 2024

And I've even documented the new tox -e test stuff. I think it is ready to merge. Agree, @gforcada ?

@gforcada
Copy link
Collaborator

gforcada commented Mar 2, 2024

@reinout sorry! I was either too busy, or my brain was melting 😓

Great job! 🌟

@gforcada gforcada merged commit 4e47834 into master Mar 2, 2024
11 checks passed
@gforcada gforcada deleted the config-with-default-template-5d3e918e branch March 2, 2024 12:12
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.

3 participants