search

plone / plone.recipe.zope2instance

zc.buildout recipe to setup and configure a Zope 2 instance.
https://pypi.org/project/plone.recipe.zope2instance
7 stars 23 forks source link

Fix resource warning. #178

Closed icemac closed 3 years ago

icemac commented 3 years ago

Fixes #176

mister-roboto commented 3 years ago

@icemac thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

icemac commented 3 years ago

@jenkins-plone-org please run jobs

icemac commented 3 years ago

I see no need for a change log entry for such a minor fix in the tests. I ran tox locally on Python 3.8 and do not see the ResourceWarning any more.

mister-roboto commented 3 years ago

@icemac thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

mauritsvanrees commented 3 years ago

I am agreed that no changelog entry is needed. And all Jenkins tests passed, but the package is not tested there.

Locally, tox -e py39 fails, but that is because it tries to install Zope 4. The GitHub Actions have been setup to do this correctly. Should be doable to fix tox.ini, but that is not for this PR.

Actions are not triggered yet. I have triggered them manually now for this branch. Let's see. Can be merged if they are green.

mauritsvanrees commented 3 years ago

The actions are green, I merge.