Hi!

One key requirement of pulling in changes is that it won't affect existing behaviour. From a quick glance, it looks like this will break all existing plugins as it goes from assuming no return to requiring a positive return 😢

I'll have a deeper look when I have some time an tidy it up.

Hi @idefix6 - can you state your agreement to the Contributor License Agreement v1.0 in a comment here please.

Hi @idefix6 - can you state your agreement (or objections) to the Contributor License Agreement v1.0 in a comment here please.

Just my humble 2 cents: This CLA states nothing. All terms are already part of the GPL. You can neither modify nor even use ViMbAdmin without accepting those terms. Moreover, this CLA actually restricts your (Open Solutions) posssibility to license ViMbAdmin under a later version of the GPL in the future: The current code is "GPLv3 or any later version", the CLA states "GPLv3" only. Don't use CLAs. Many developers will be put off. http://ebb.org/bkuhn/blog/2014/06/09/do-not-need-cla.html

Am 28.03.2015 um 13:54 schrieb barryo:

Hi @idefix6 https://github.com/idefix6 - can you state your agreement
to the Contributor License Agreement
https://github.com/opensolutions/ViMbAdmin/wiki/Contributor-License-Agreement
v1.0 in a comment here please.

I agree.

Gruß
Matthias

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the universe trying to
produce bigger and better idiots. So far, the universe is winning." --
Rich Cook

Hi Barry,

Am 28.03.2015 um 13:45 schrieb barryo:

One key requirement of pulling in changes is that it won't affect
existing behaviour. From a quick glance, it looks like this will break
all existing plugins as it goes from assuming no return to requiring a
positive return 😢

I'll have a deeper look when I have some time an tidy it up.

do you have plugins that are using hooks?
I have a standard config running and found no problems yet (but maybe I
do not use all plugins).

But you are right, the plugins must be modified to return a true in
there hooks.
If a hook is not existing, commit:
mfechner@eb6a46e
ensures that always true is returned, but that matched only for not
existing hooks.

Gruß
Matthias

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs, and the universe trying to
produce bigger and better idiots. So far, the universe is winning." --
Rich Cook

I just had a look into the changes again and it only has an effect on plugins that will implement an alias PreRemove hook.
I checked all plugins and no one is implementing such a hook, so the modification will not break anything. Maybe we should add to the documentation that a Pre- hook must always return a true value if it was successfully executed to enable the Pre- checks.

Maybe the steps we could do are:

1. Update documentation, pre hooks must always return a true if they were executed successfully
2. Pull my request, that will only effect the Aliases PreDelete hook
3. Update all PreDelete implementations to return a true value
4. Modify all other controller to check the return value

I think this has the lowest impact and would reduce the risk to really break anything by accident.
What do you think?

Dear Barry,

any chance to get the modification I did 2 years ago part of the master branch?
This first modification should not have an impact to any existing module.

@idefix6 - done with a little tidying up and better retention of existing logic.

@barryo sorry, your tidy up broke all my pending changes I would like to merge, the intension here was to get my modifications as clearly and savely as possible into master branch. I will rewrite all my pending branches to get it alligned with your tidy-up.

Sorry @idefix6 - I hope you can see what my intention was? Essentially: code works just as expected for everyone else.

Dear @barryo no problem, I cleaned up my modifications and send a new pull request to get a new hook available.