daledavies / moodle-tool_registrationrules

GNU General Public License v3.0
2 stars 1 forks source link

Fix rule plugins #55

Open daledavies opened 1 month ago

daledavies commented 1 month ago

I'd probably recommend reviewing this commit by commit, rather than all at once.

daledavies commented 1 week ago

@phager-at Just need to fix the failing checks and it'll be back over to you.

daledavies commented 1 week ago

All done 😃

phager-at commented 2 days ago

I've added some more thoughts about general architecture, as I wasn't able to work on it while you implemented your last changes (roughly 2 weeks ago).

TL;DR:

daledavies commented 1 day ago

I think I have a solution that satisfies all your feeedback so far.

Now we hopefully have the best of both worlds, interfaces for everything and rule plugin definition can still be quite simple.

Just tidying up and I'll push as a new commit so you can see the difference.

daledavies commented 1 day ago

I'm not too pleased with so many getters and setters, but I guess that's a failing of PHP. Maybe one day we can use property hooks or something!

daledavies commented 13 hours ago

@phager-at As mentioned above, I've taken your suggestions into account and reworked things in the latest commit. Hopefully this is better, I'm certainly happier with it - thanks for your advise.

Back over to you :)

daledavies commented 12 hours ago

Rebased to fix conflicts now main has been updated.