Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tool_s3logs\client is not a valid namespace #5

Closed
mudrd8mz opened this issue Mar 29, 2019 · 5 comments
Closed

tool_s3logs\client is not a valid namespace #5

mudrd8mz opened this issue Mar 29, 2019 · 5 comments

Comments

@mudrd8mz
Copy link

Moodle has certain rules on how namespaces are selected. As per https://docs.moodle.org/dev/Coding_style#Rules_for_level2 your current namespace tool_s3logs\client should be tool_s3logs\local\client.

@dmitriim
Copy link
Member

dmitriim commented Jun 17, 2019

Hi @mudrd8mz !

I have checked a couple of open source plugins that have in our git repo and they all do not follow this recommendation/rule. Is it something new? Do I understand correctly if you want to organise your files in folders, then you need to have classes/local/folder1 classes/local/folder2 and not classes/lfolder1 As long as folderN is not part of the core? I find this a bit confusing.

Also would be probably good to add this case as an example/reference to https://docs.moodle.org/dev/Automatic_class_loading#Namespaces

@dmitriim
Copy link
Member

fixed anyway

@mudrd8mz
Copy link
Author

As far as I can see, the https://docs.moodle.org/dev/Automatic_class_loading#Namespaces has a note about it:

Note there are restrictions on valid choices for the first and second level namespaces in Moodle - see Coding_style#Namespaces for more info

@brendanheywood
Copy link

That is something I have very rarely seen done right (including my code). This should be something that fails CI loudly, I've opened that here:

open-lms-open-source/moodle-plugin-ci#97

@dmitriim
Copy link
Member

dmitriim commented Jun 17, 2019

Yeah. I've been doing plugin dev for ages and somehow this slipped out from me. I'll be pointing to this during my code reviews. @mudrd8mz thanks for heads up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants