Deprecate (and eventually remove) implprefix support

[goodboy]

As per the question in #114 I think we should now deprecate this undocumented implprefix feature so we can remove it in the next major release.

[nicoddemus]

Unless I'm missing something in this comment, nearly every pytest plugin still relies on the pytest_ prefix naming convention for hook implementation, so I don't think we can't remove that support for pytest without breaking every plugin in existence. 😬

I suspect that the same is true for devpi (see devpi-jenkins), but seems other tox plugins use the @hookimpl decorator (tox-durations, tox-venv), but this is not an exhaustive search.

[goodboy]

so I don't think we can't remove that support for pytest without breaking every plugin in existence.

@nicoddemus right so can we add a deprecation warning first and then move forward with removal later?
I'm definitely not advocating we pull it out without first making sure we don't break existing code 👍

Also I'd be partial to instead allowing for a prefix kwarg to PluginManger.register to accomplish this in pytest for now. It'd be basically the same except we don't require it at creation time and instead at register() invocation time.

[nicoddemus]

right so can we add a deprecation warning first and then move forward with removal later?

To be perfectly honest I'm not willing to break every pytest plugin out there without a very compelling reason, this will make for some very angry pytest users and plugin authors alike. 😬 Not every pytest plugin is actively maintained anymore but still works after all those years.

If only pytest still uses implprefix, I would suggest for us to move the implprefix implementation over to pytest and remove it from pluggy eventually, but since devpi also uses it this complicates this a little bit more. devpi has only a few plugins so this might be feasible if devpi folks agree.

cc @fschulze @obestwalter @asottile @gaborbernat

[nicoddemus]

(forgot to mention, this will break not only plugins released on PyPI but also user-defined plugins in conftest.py files)

[goodboy]

Not every pytest plugin is actively maintained anymore but still works after all those years.

Hehe yeah alright. Let's see what the consensus is 👍

this will break not only plugins released on PyPI but also user-defined plugins in conftest.py files

To start it should only issue a warning imho.

[nicoddemus]

Sure, no problem. I think it is healthy for new APIs such as pluggy to remove their historic clutches via deprecation/removal, but for pytest this feature specifically I think this is not possible. I'm perfectly happy to move the support over to pytest though. 😁

[asottile]

From a tox perspective, it seems we don't use this option: https://github.com/tox-dev/tox/blob/6b16b095f8b603c23aefe8155cb4e6867db8c178/tox/config.py#L50

Though I do note that tox's pytest plugin (which I believe is only used internally?) uses this feature : https://github.com/tox-dev/tox/blob/6b16b095f8b603c23aefe8155cb4e6867db8c178/tox/_pytestplugin.py#L21

I wouldn't be surprised if pytest (and pytest plugins) is the only consumer of this feature -- though if others come forth I'd lean towards keeping it -- it's only ~4 lines to maintain after all 😆 (though then you'd probably want to document it I guess?).

[fschulze]

It should definitely throw a deprecation warning. I can then remove it's use from devpi more easily 😉

[hpk42]

On Fri, Jan 05, 2018 at 10:01 +0000, Florian Schulze wrote: It should definitely throw a deprecation warning. I can then remove it's use from devpi more easily 😉

Maybe throw a deprecation warning and add a little info on how pluggy-using projects can implement "implprefix" on their own? When it gets removed there could be an additional links to pytest code which real-life implements this extra.

[nicoddemus]

It should definitely throw a deprecation warning. I can then remove it's use from devpi more easily

Great!

I think we should go ahead and deprecate it then, while also moving/duplicating the functionality in pytest at the same time to avoid pytest users from getting deprecation warnings. The deprecation should definitely go into a minor release (currently pytest pins to pluggy>=0.5,<0.7).

[RonnyPfannschmidt]

i propose decomposing the implementation in pluggy - this can be done in a way that we keed the old behaior for a while and expect consuming projects to change the used names to something that ensures functions,
afterwards it can be moved

[goodboy]

i propose decomposing the implementation in pluggy

@RonnyPfannschmidt Can you be a little more specific?

[RonnyPfannschmidt]

i would like to apply the strategy pattern to discovering hooks in a plugin

that way we can support more explicit mechanisms in a relatively sane manner and the old behaviour could be isolated, unittested and left here

[goodboy]

^ created a PR to deprecate which will be followed by #145 to address removal and the possible strategy pattern approach mentioned by @RonnyPfannschmidt.