plone / guillotina

Python AsyncIO data API to manage billions of resources
https://guillotina.readthedocs.io/en/latest/
Other
187 stars 50 forks source link

Deleting import zope.interface implements #1189

Closed nilbacardit26 closed 7 months ago

nilbacardit26 commented 7 months ago

Every time I install guillotina as a dependency I encounter this error: Error importing plugin "guillotina.tests.fixtures": cannot import name 'implements' from 'zope.interface. implements is not there anymore in version 6.1 'zope.interface

I do not know what this line does, implements is not used anywhere. I passed all the test without this line. Some of you know why is it there? https://github.com/plone/guillotina/blob/master/guillotina/component/interfaces.py#L19

codecov-commenter commented 7 months ago

Codecov Report

Merging #1189 (ef0ae70) into master (29a6958) will decrease coverage by 0.0%. The diff coverage is n/a.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/plone/guillotina/pull/1189/graphs/tree.svg?width=650&height=150&src=pr&token=MIUJDWnGXD&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=plone)](https://app.codecov.io/gh/plone/guillotina/pull/1189?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=plone) ```diff @@ Coverage Diff @@ ## master #1189 +/- ## ======================================== - Coverage 94.6% 94.6% -0.0% ======================================== Files 377 377 Lines 32771 32770 -1 ======================================== - Hits 30992 30991 -1 Misses 1779 1779 ``` | [Files](https://app.codecov.io/gh/plone/guillotina/pull/1189?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=plone) | Coverage Δ | | |---|---|---| | [guillotina/component/interfaces.py](https://app.codecov.io/gh/plone/guillotina/pull/1189?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=plone#diff-Z3VpbGxvdGluYS9jb21wb25lbnQvaW50ZXJmYWNlcy5weQ==) | `100.0% <ø> (ø)` | |
vangheem commented 7 months ago

It's a vanity import to be able to use the implements interface. If that is no longer used/support, remove the import!

I don't think it's a good idea to hard pin a version -- it can be annoying for consumers

nilbacardit26 commented 7 months ago

@vangheem done, I will run the tests I have in another project just to be sure this import is not breaking anything

vangheem commented 7 months ago

@nilbacardit26 everything look good?

We can merge then

nilbacardit26 commented 7 months ago

@vangheem yes, I am not authorized to merge, feel free to do it