moodle-tool_mergeusers icon indicating copy to clipboard operation
moodle-tool_mergeusers copied to clipboard

Published 20 hours ago • verified publisher icon jpahullo

autoloading

Open danmarsden opened this issue 10 years ago • 6 comments

Moodle has it's own built-in class autoloading, it would be better for the code to follow this standard rather than implement it's own class autoloading.

danmarsden avatar Sep 10 '14 08:09 danmarsden

here's an example of using core autoloading for the config class; https://github.com/danmarsden/moodle-tool_mergeusers/commit/2fecc11a5b7197d25323845cd9ede297fbd2a21a

and here are the related moodledocs: https://docs.moodle.org/dev/Automatic_class_loading

danmarsden avatar Sep 10 '14 08:09 danmarsden

Thanks for your comments!

We'll take a look at that. Meanwhile, you could build a PR with the specific changes too.

Thanks in advance!

Jordi

jpahullo avatar Sep 10 '14 19:09 jpahullo

Thanks Jordi - I might not have time at this stage - was making the changes as part of a review for a client - we failed the review on the performance of settings.php and the generic class name "config" but after fixing those issues we may not have much time to do anything else.

danmarsden avatar Sep 10 '14 23:09 danmarsden

Thanks anyway!

Keep tuned in!

Jordi

jpahullo avatar Sep 11 '14 10:09 jpahullo

We have addressed this issue in the short past.

However, files in mergeusers/classes/ should be:

  • config.php and logger.php for M2.6 and on, BUT
  • tool_mergeusers_config.php and tool_mergeusers_logger.php for M2.4 and M2.5.

Therefore, we have updated the last version of this plugin not compatible for M2.6 and on, actually.

I think we should start working on branching MOODLE_XX_STABLE or something like that.

Jordi

jpahullo avatar Feb 25 '15 17:02 jpahullo

This still applies. We could start moving current lib classes into classes/ directory properly. It would be easier to develop with this plugin and all its components.

jpahullo avatar Apr 17 '23 16:04 jpahullo