collective / collective.logbook

Advanced exception logging for Plone
5 stars 2 forks source link

Plone 5 compatible version of logbook ? #2

Closed glemignot closed 7 years ago

glemignot commented 7 years ago

Hello,

We would need a version of logbook compatible with Plone 5.

Is there any current effort for porting logbook to Plone 5 (so we don't duplicate effort) ?

And is there any official policy about Plone version support ? Ie, should the same version of logbook support both Plone 4.x and Plone 5.x, or can we make a new version that is Plone 5.x only ?

From a quick glance, the most important change between Plone 4.x and 5.x for logbook is the "Plone control panel" hooking mechanism.

Regards,

ramonski commented 7 years ago

Hi @glemignot, Thanks for your Request.

Currently there is no effort of my side for porting collective.logbook to Plone 5, so I would highly appreciate if you provide such a version.

Please feel free to make an explicit version supporting only Plone 5, although a version which supports Plone 4.x and 5.x would be nice.

I also think that the Plone control Panel hookup is the only thing to change, as the rest should be quite generic for all versions since Plone 3.x to Plone 5.x.

Thanks and just contact me if you have any further questions.

glemignot commented 7 years ago

I did a first shot at porting on https://github.com/glemignot/collective.logbook . It seems to be working (although I never used the Webhook feature, so didn't test it).

There is one known issue left, the hook "Enable Logbook logging" boolean is no longer there, since the storing of options changed (from properties to registry) I'll have to rehook that differently.

Any feedback is welcome.

ramonski commented 7 years ago

Thanks @glemignot,

the changes look fine to me (deleting obsolete code is always good).

No worries about the hook, leave it away. It was anyhow a little bit hacky to do it like this to activate/revert the monkey patch. Just update the documentation accordingly, please.

Regarding the webhooks, neither do I used them before. So I would recommend to bring out a new version for Plone 5 soon and then wait until people use it and give feedback/complaints.

If you like I can add you to the Pypi release managers for this package, just give me your Pypi username.

Thanks

glemignot commented 7 years ago

Hello,

I tried to get the "logging enabled" settings to still work at startup, but the Plone registry isn't available at Zope 2 initialize() point, so it doesn't work.

For now I commented out the settings, and updated the documentation, making version 0.8b2.

To me it's sufficient for release, but up to you to decide. As for the release itself, it's probably easier if you do it yourself, but if you want my (newly created) pypi account is glemignot.

Regards,

ramonski commented 7 years ago

Welcome on PyPI as a release manager of collective.logbook.

Thanks for your efforts @glemignot. I thought already that that the ZCA won't be available at this time, I think that was the reason I added the property on the Zope root. So no worries, if the documentation is updated, everything is fine.

I'm looking forward to see this version released.

Bests, Ramon

ramonski commented 7 years ago

Please don't forget to merge your code to this repository as a PR (you can accept/merge it yourself), Thanks.

glemignot commented 7 years ago

Thanks !

I made a pull request, but I don't have the rights to merge it, could you merge it and then I'll do the release from here ?

glemignot commented 7 years ago

And here it is : https://pypi.python.org/pypi/collective.logbook/0.8b2

ramonski commented 7 years ago

Awesome! Good work!

Only the restructured text seems to be unformatted on pypi.

Maybe you find the error with the following command: python setup.py check --restructuredtext --strict --metadata

Thanks

glemignot commented 7 years ago

There was indeed some RST formatting issues, I made a new pull request with a fix (but I probably did something wrong github-related, since it seems to want to merge everything again... I hope it's easy to fix).