Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Component/s: Coding style
    • Labels:

      Description

      We need to define the rules for use of namespaces and escaping.

      Eloy put it very well in MDL-39854:

      While I don't oppose to this, I think this requires way more detailed documentation about the uses we are expecting and the do-s and don't-s with them. Note this is apart from this issue itself, but if we officially start supporting them, because of their nice autoloading capabilities, they need to be rulez in the docs/codings style/code checker. We should not allow, for sure, things like:
      bracketed nomenclature
      multiple namespaces (or classes) per file
      incorrect (non component) namespace prefixes. for example "\superduper".
      incorrect (non subsystem) namespace suffixes. for example "\mod_forum\inventedbyme" or "\core\becauseiwant".

        Gliffy Diagrams

          Issue Links

            Activity

            Show
            poltawski Dan Poltawski added a comment - Some links: http://www.php.net/manual/en/language.namespaces.fallback.php http://www.php.net/manual/en/language.namespaces.rules.php
            Hide
            poltawski Dan Poltawski added a comment -

            One point of debate which has already come up is escaping of function names.

            Show
            poltawski Dan Poltawski added a comment - One point of debate which has already come up is escaping of function names.
            Hide
            skodak Petr Skoda added a comment -

            1/ the top most namespace must use frankenstyle component name (plugintype_pluginname, core_subsystem or core)
            2/ the location for class files is strictly defined by the classloader
            3/ there should be at least one sub namespace so that the files do not mix with non-namespaced classes
            4/ new subsystems with many files should go to namespaces - this prevents pollution in lib/classes
            5/ namespaces give huge benefits when implementing some shared features in plugins - you can use the same class name in all plugins

            Show
            skodak Petr Skoda added a comment - 1/ the top most namespace must use frankenstyle component name (plugintype_pluginname, core_subsystem or core) 2/ the location for class files is strictly defined by the classloader 3/ there should be at least one sub namespace so that the files do not mix with non-namespaced classes 4/ new subsystems with many files should go to namespaces - this prevents pollution in lib/classes 5/ namespaces give huge benefits when implementing some shared features in plugins - you can use the same class name in all plugins
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            A simple, example/question:

            Q001) In plugin xxx_yyy I want to implement feature zzzz using an autoloaded class.

            1) So it's clear that I create the "classes" dir under xxx_yyy "home". And then the "zzzz.php" file there. No discussion there. But…

            2) Which of this is the preferred way to create that class:

            • use namespace "\xxx_yyy" and name the class "zzzzz", or
            • don't use namespace and name the class "xxx_yyy_zzzzz" ?

            In other words, for top-most component classes… do we use namespaces or no? I can see clearly that for non-top classes (those belonging to an "API/category" (like "event"), we must use namespaces. But topmost?

            Q002) Following the previous example… how to we restrict use of sub-namespaces?

            • Are we going to accept complete freedom there like, for example "\mod_forum\becauseiwant"… or
            • That last element in the namespace should be validated to be a correct API/category always (like "event"…).

            Q003) What to do with global scope functions. Do we a) require, b) allow, c) forbid backslashing them when within a namespace?

            Q004) What happens with the rest of the PHP namespace implementation:

            • Do we allow to use "use" a) yes, b) no and, if yes, how (with alias, without alias…).
            • Do we allow multiple namespaces per file, and bracketed ones

            Just labeling the Qs for future reference.

            Edited: Sorry, fixing 2 typos in the questions. \xxx\yyy occurrences should be all \xxx_yyy

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited A simple, example/question: Q001) In plugin xxx_yyy I want to implement feature zzzz using an autoloaded class. 1) So it's clear that I create the "classes" dir under xxx_yyy "home". And then the "zzzz.php" file there. No discussion there. But… 2) Which of this is the preferred way to create that class: use namespace "\xxx_yyy" and name the class "zzzzz", or don't use namespace and name the class "xxx_yyy_zzzzz" ? In other words, for top-most component classes… do we use namespaces or no? I can see clearly that for non-top classes (those belonging to an "API/category" (like "event"), we must use namespaces. But topmost? Q002) Following the previous example… how to we restrict use of sub-namespaces? Are we going to accept complete freedom there like, for example "\mod_forum\becauseiwant"… or That last element in the namespace should be validated to be a correct API/category always (like "event"…). Q003) What to do with global scope functions. Do we a) require, b) allow, c) forbid backslashing them when within a namespace? Q004) What happens with the rest of the PHP namespace implementation: Do we allow to use "use" a) yes, b) no and, if yes, how (with alias, without alias…). Do we allow multiple namespaces per file, and bracketed ones Just labeling the Qs for future reference. Edited: Sorry, fixing 2 typos in the questions. \xxx\yyy occurrences should be all \xxx_yyy
            Hide
            skodak Petr Skoda added a comment -

            Q001/

            topmost namespace is using flankestyle component - for example \mod_forum\event*, \core\event*

            old and simple things without namespace, new complex things with namespace

            Q002/

            for now subnamespaces should be probably used only for features that are implemented in multiple plugins

            "\mod\forum\xxx" in not acceptable, "mod" is not frankenstyle component name

            Q003/

            you do not need to backslash global function names usually, there should not be many collisions with class names; in any case we should not be introducing new API using functions

            Q004/

            I do not see a big need for strict rules related to use and backslashing - good IDEs warn you when you do it incorrectly.

            Show
            skodak Petr Skoda added a comment - Q001/ topmost namespace is using flankestyle component - for example \mod_forum\event*, \core\event* old and simple things without namespace, new complex things with namespace Q002/ for now subnamespaces should be probably used only for features that are implemented in multiple plugins "\mod\forum\xxx" in not acceptable, "mod" is not frankenstyle component name Q003/ you do not need to backslash global function names usually, there should not be many collisions with class names; in any case we should not be introducing new API using functions Q004/ I do not see a big need for strict rules related to use and backslashing - good IDEs warn you when you do it incorrectly.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            One more question, slightly related with this:

            Q005) For code clearly "belonging" to a subsystem, are we going to use the subsystem/classes directory or are we phasing out subsystems and code will go to lib/classes ? Example, current event classes, all under lib/classes. Are they a exception or are they the way?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - One more question, slightly related with this: Q005) For code clearly "belonging" to a subsystem, are we going to use the subsystem/classes directory or are we phasing out subsystems and code will go to lib/classes ? Example, current event classes, all under lib/classes. Are they a exception or are they the way?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            For reference, a real example of my Q0001, in a subsystem, not in a plugin:

            https://github.com/moodle/moodle/tree/master/course/classes

            1) topmost classes are not using namespaces at all, so they are "core_course_zzzzzz" (aka, they are using the 2nd way defined in Q0001. No problem with that).

            2) But then, we have an invented a sub-namespace, named "core_course\management" and 1 helper class on it. Shouldn't it be also core_course_managementhelper instead? Or if we agree that "management" is a correct sub-namespace (I don't think so) shouldn't all them be using it?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - For reference, a real example of my Q0001, in a subsystem, not in a plugin: https://github.com/moodle/moodle/tree/master/course/classes 1) topmost classes are not using namespaces at all, so they are "core_course_zzzzzz" (aka, they are using the 2nd way defined in Q0001. No problem with that). 2) But then, we have an invented a sub-namespace, named "core_course\management" and 1 helper class on it. Shouldn't it be also core_course_managementhelper instead? Or if we agree that "management" is a correct sub-namespace (I don't think so) shouldn't all them be using it?
            Hide
            skodak Petr Skoda added a comment -

            I am against \core_xxx* namespaces for things like events. I vote for putting as much as possible to lib/classes/xxx/ namespaces, course management stuff feels that it could be actually implemented as a plugin, the core_course\management makes sense there...

            Show
            skodak Petr Skoda added a comment - I am against \core_xxx* namespaces for things like events. I vote for putting as much as possible to lib/classes/xxx/ namespaces, course management stuff feels that it could be actually implemented as a plugin, the core_course\management makes sense there...
            Hide
            samhemelryk Sam Hemelryk added a comment -

            The course management in fact started like as an admin tool, that's what it felt like when I first started working on it.
            However during development it became apparent that really that was not quite correct as we don't have any alternative means of managing that structure. As such I converted it from the admin tool to being part of core_course.

            I like namespaces and the enhanced organisation they provide.
            Really I'd vote sensible freedom, yes it would be objective on a case by case basis and for sure it would continue to be a popular point of debate. Through use we'd establish conventions based on what appeared natural to developers.
            Remembering that the autoloader imposes structural conventions already.
            I suppose consistency would require consideration perhaps a few other light rules would come to light. But really namespaces as an organisational tool.
            But perhaps am I not seeing the damage that can be done by "freedom" coding.

            That being said, if we decide to limit it then I'm for the all out draconian control measures as from an integration point of view if I've got to impose rules best they be clear, definitive, and restrictive (lets not become a law institute requiring a book full of clauses).
            Permit the use of namespaces in explicit cases: always conforming to the frankenstyle + subns category/api only + fully qualified internals (all globals get \'d)

            So really I'm casting two votes, one for black and one for white. I don't want grey to be an option.
            +1 to subjective freedom
            +0.5 to draconian rules

            Not overly helpful sorry

            Show
            samhemelryk Sam Hemelryk added a comment - The course management in fact started like as an admin tool, that's what it felt like when I first started working on it. However during development it became apparent that really that was not quite correct as we don't have any alternative means of managing that structure. As such I converted it from the admin tool to being part of core_course. I like namespaces and the enhanced organisation they provide. Really I'd vote sensible freedom, yes it would be objective on a case by case basis and for sure it would continue to be a popular point of debate. Through use we'd establish conventions based on what appeared natural to developers. Remembering that the autoloader imposes structural conventions already. I suppose consistency would require consideration perhaps a few other light rules would come to light. But really namespaces as an organisational tool. But perhaps am I not seeing the damage that can be done by "freedom" coding. That being said, if we decide to limit it then I'm for the all out draconian control measures as from an integration point of view if I've got to impose rules best they be clear, definitive, and restrictive (lets not become a law institute requiring a book full of clauses). Permit the use of namespaces in explicit cases: always conforming to the frankenstyle + subns category/api only + fully qualified internals (all globals get \'d) So really I'm casting two votes, one for black and one for white. I don't want grey to be an option. +1 to subjective freedom +0.5 to draconian rules Not overly helpful sorry
            Hide
            damyon Damyon Wiese added a comment -

            I do not think we should be prescriptive about the namespace sub levels. If I write a particularly complex plugin, I should be able to use the sub namespaces to structure my code logically.

            I would prefer the use of namespaces over prefixes e.g. \assignfeedback_editpdf\renderer (this style is also easier to validate with codechecker) - but I can't use that currently, it has to be \assignfeedback_editpdf_renderer because the renderer functions don't really support the autoloading well yet.

            I give a strong -1 (-2 even) for e.g. \get_string . We should have a clear rule, no functions in namespaces, only classes so there is never ambiguity.

            Show
            damyon Damyon Wiese added a comment - I do not think we should be prescriptive about the namespace sub levels. If I write a particularly complex plugin, I should be able to use the sub namespaces to structure my code logically. I would prefer the use of namespaces over prefixes e.g. \assignfeedback_editpdf\renderer (this style is also easier to validate with codechecker) - but I can't use that currently, it has to be \assignfeedback_editpdf_renderer because the renderer functions don't really support the autoloading well yet. I give a strong -1 (-2 even) for e.g. \get_string . We should have a clear rule, no functions in namespaces, only classes so there is never ambiguity.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            +10 for always use (i really want to limit the vocabulary):

            1) level 1: component name (frankenstyle, mod_forum, core_course, core...).
            2) level 2: valid API. Nothing nor being an API (http://docs.moodle.org/dev/Core_APIs) can be in level 2. Another (correct) example: MDL-25500
            3) level 3 and down: freedom.

            That implies:

            1) level 1 will match @package or "core".
            2) level 2 will match @category

            Pretty perfect. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - +10 for always use (i really want to limit the vocabulary): 1) level 1: component name (frankenstyle, mod_forum, core_course, core...). 2) level 2: valid API. Nothing nor being an API ( http://docs.moodle.org/dev/Core_APIs ) can be in level 2. Another (correct) example: MDL-25500 3) level 3 and down: freedom. That implies: 1) level 1 will match @package or "core". 2) level 2 will match @category Pretty perfect. Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Eloy,

            I really like the idea of making this as prescribed as possible, because I think it makes it simplest for people creating things.

            But as I was just setting up a vote for the iTeam to do it, I realised that I just don't think that enforcing level 2 to core apis can work well. I can imagine an activity module has a lot of 'business logic' which does not fit under our core apis. In fact, it will be creating its own classes which then call the core apis rather than necessarily matching it.

            Just a quick grep of mod_assign for example - I don't know where assignfeedback or assignsubmission would naturally fit under our core apis. (dml??)

            Show
            poltawski Dan Poltawski added a comment - Hi Eloy, I really like the idea of making this as prescribed as possible, because I think it makes it simplest for people creating things. But as I was just setting up a vote for the iTeam to do it, I realised that I just don't think that enforcing level 2 to core apis can work well. I can imagine an activity module has a lot of 'business logic' which does not fit under our core apis. In fact, it will be creating its own classes which then call the core apis rather than necessarily matching it. Just a quick grep of mod_assign for example - I don't know where assignfeedback or assignsubmission would naturally fit under our core apis. (dml??)
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Dan, that's exactly the problem I want to avoid. I don't want people to start "registering" namespaces like crazy.

            Both "feedback" or "submission" are two potential candidates to have an unified API some day. Not saying they are going to be created but it's possible (conceptually). Having them already used will lead to problems.

            Also, what's the problem with "assignfeedback" and "assignsubmission". Talking about them as classes, they can fit perfectly under \mod_assign namespace.

            Talking about them as subplugins, they own their \assignfeedback_xxx and \assignsubmission_xxx namespaces.

            And talking about them as "plugininfo" subclasses, it's ok, as far as "plugininfo" is (should be) a core API as far as it's shared by all plugin types.

            Has perfect sense... or am I missing anything?

            More yet, if for some reason, still there is any case where the rule for 2nd level does not fit, we always can create a "hodgepodge" API (\assignfeedback_xxx\hodgepodge\liberty for example) for things not belonging to any API. But I'm still asking for some valid example justifying its creation.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Dan, that's exactly the problem I want to avoid. I don't want people to start "registering" namespaces like crazy. Both "feedback" or "submission" are two potential candidates to have an unified API some day. Not saying they are going to be created but it's possible (conceptually). Having them already used will lead to problems. Also, what's the problem with "assignfeedback" and "assignsubmission". Talking about them as classes, they can fit perfectly under \mod_assign namespace. Talking about them as subplugins, they own their \assignfeedback_xxx and \assignsubmission_xxx namespaces. And talking about them as "plugininfo" subclasses, it's ok, as far as "plugininfo" is (should be) a core API as far as it's shared by all plugin types. Has perfect sense... or am I missing anything? More yet, if for some reason, still there is any case where the rule for 2nd level does not fit, we always can create a "hodgepodge" API (\assignfeedback_xxx\hodgepodge\liberty for example) for things not belonging to any API. But I'm still asking for some valid example justifying its creation. Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            Hmm, well I don't think I share your desire to control the levels below 1 (component) that much. Why is it important that we get involved there? I think that will make naming unnatural for many plugin authors.

            For core, I agree with second level being enforced that way. But I think it is introducing an artificial layer for many of the plugins and they are free to keep their own mess under their theme_wood\ space

            Show
            poltawski Dan Poltawski added a comment - Hmm, well I don't think I share your desire to control the levels below 1 (component) that much. Why is it important that we get involved there? I think that will make naming unnatural for many plugin authors. For core, I agree with second level being enforced that way. But I think it is introducing an artificial layer for many of the plugins and they are free to keep their own mess under their theme_wood\ space
            Hide
            poltawski Dan Poltawski added a comment -

            So this is my proposal...

            Q001) Always use component/core api namespace.
            use \frankenstyle;
            I think this is worth enforcing as a rule because it makes clear rules about which namespace you are in in any file you are modifying. It should help people not get caught out by differing use of namespaces. The nature of work across Moodle means often we are making a couple of lines change in many different files..

            Q002) For levels:

            • Level1: Restrict top level to component
            • Level2:
              • Enforce use of core api for core..
              • Recommend use of core api for plugins.

            Q003) Forbid blackslashing of global functions. Rationale:

            • I think it helps people understand the php rules about it better. Classes will break if not backslashed, global functions mostly wont. If people sometimes backslash functions and sometimes don't, I think it'll be confusing.
            • I hate the backslashes and (seriously) I think it helps train the eye to look for for them only when they are needed.

            Q004) One namespace per file - (to fit with Q1).

            Show
            poltawski Dan Poltawski added a comment - So this is my proposal... Q001) Always use component/core api namespace. use \frankenstyle; I think this is worth enforcing as a rule because it makes clear rules about which namespace you are in in any file you are modifying. It should help people not get caught out by differing use of namespaces. The nature of work across Moodle means often we are making a couple of lines change in many different files.. Q002) For levels: Level1: Restrict top level to component Level2: Enforce use of core api for core.. Recommend use of core api for plugins. Q003) Forbid blackslashing of global functions. Rationale: I think it helps people understand the php rules about it better. Classes will break if not backslashed, global functions mostly wont. If people sometimes backslash functions and sometimes don't, I think it'll be confusing. I hate the backslashes and (seriously) I think it helps train the eye to look for for them only when they are needed. Q004) One namespace per file - (to fit with Q1).
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Sorry Dan, I must confess I don't understand some of your replies. Not arguing they are incorrect at all, but found them incomplete/unrelated with my initial Q00x exposition.

            Could you clarify:

            in Q001: Why are you naming namespace imports there (aka, "use")? I think the original question was not about that. What means "Always use component/core api namespace." ? Sorry.

            in Q002: Could you clarify what is "core" and what's "plugins" for you? I mean, does "core" include "core/standard plugins" and your reference to "plugins" means "contrib add-ons". Or are you saying "core" as only the "core" component, with or without subsystems? Needs clarification.

            in Q004: There was some more points to decide there. # of namespaces (I agree 1), utilization of aliases and/or not aliased imports (aka "use"). I feel we should use only non-aliased ones, in fact I arrived here with an example commit where "use's use" (sorry could not resist) may lead to more readable code)

            Let's keep the ball moving. Ciao

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Sorry Dan, I must confess I don't understand some of your replies. Not arguing they are incorrect at all, but found them incomplete/unrelated with my initial Q00x exposition. Could you clarify: in Q001: Why are you naming namespace imports there (aka, "use")? I think the original question was not about that. What means "Always use component/core api namespace." ? Sorry. in Q002: Could you clarify what is "core" and what's "plugins" for you? I mean, does "core" include "core/standard plugins" and your reference to "plugins" means "contrib add-ons". Or are you saying "core" as only the "core" component, with or without subsystems? Needs clarification. in Q004: There was some more points to decide there. # of namespaces (I agree 1), utilization of aliases and/or not aliased imports (aka "use"). I feel we should use only non-aliased ones, in fact I arrived here with an example commit where "use's use" (sorry could not resist) may lead to more readable code) Let's keep the ball moving. Ciao Ciao
            Hide
            emerrill Eric Merrill added a comment -

            I've been in the process of writing some contrib plugins using the new namespace system, so I want to throw in my 2 cents in relation to Q002:

            Please please please let (contrib) devs use sub-name spaces as they see fit. Being able to do something like \mod_assign\becauseiwant\file.php makes a plugin much easier to maintain and work on. Yes, there is some down the road risk of a collision with a Moodle core API - but contribs have to be maintained to stay working with core anyways. Personally I would love it if all Moodle API classes moved to a dedicated directory, like \mod_assign\api\events so they are well segregated and you don't have to worry about collisions in the future, but I know that isn't going to happen...

            Show
            emerrill Eric Merrill added a comment - I've been in the process of writing some contrib plugins using the new namespace system, so I want to throw in my 2 cents in relation to Q002: Please please please let (contrib) devs use sub-name spaces as they see fit. Being able to do something like \mod_assign\becauseiwant\file.php makes a plugin much easier to maintain and work on. Yes, there is some down the road risk of a collision with a Moodle core API - but contribs have to be maintained to stay working with core anyways. Personally I would love it if all Moodle API classes moved to a dedicated directory, like \mod_assign\api\events so they are well segregated and you don't have to worry about collisions in the future, but I know that isn't going to happen...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Just arguing with the opposite, I'd be happy if contrib plugins (add-ons) do not pollute core space and I'd be happy recommending all them to be under "\mod_mymod\addon[api]\becauseiwant\iorganize\things\thisway".

            In any case, this is 100% about core. With core being core + core subsystems + core plugins (bundled with Moodle distro).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Just arguing with the opposite, I'd be happy if contrib plugins (add-ons) do not pollute core space and I'd be happy recommending all them to be under "\mod_mymod\addon [api] \becauseiwant\iorganize\things\thisway". In any case, this is 100% about core. With core being core + core subsystems + core plugins (bundled with Moodle distro).
            Hide
            emerrill Eric Merrill added a comment - - edited

            To me, the discussion is still the same - there is no reason that mod_quiz and mod_contribthingy shouldn't follow the same set of rules. Personally, I try to write my (newer) code to core standards. Yes, you don't have as much control over it, but that doesn't mean these rules have no bearing over contrib code. And contrib devs do have a stake in this discussion.

            It seems weird that \mod_contribthingy\ * would be considered core namespace, even just by nomenclature.

            Show
            emerrill Eric Merrill added a comment - - edited To me, the discussion is still the same - there is no reason that mod_quiz and mod_contribthingy shouldn't follow the same set of rules. Personally, I try to write my (newer) code to core standards. Yes, you don't have as much control over it, but that doesn't mean these rules have no bearing over contrib code. And contrib devs do have a stake in this discussion. It seems weird that \mod_contribthingy\ * would be considered core namespace, even just by nomenclature.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Eric, I agree contrib code should follow core rules. Just they are not enforced there, just recommended. And yes, contrib authors have a stake (and a steak) in this discussion. Nothing to object.

            Back to Q0002) I think there is already an agreement about level1 being always the component name. Nothing to discuss there. More yet,

            P0) Developers already own level1, so any class can be added there (namespaced or not namespaces, that's Q0001. There is complete freedom there (again with Q0001 to decide).

            Problem comes with level2, where there are these options (unless I'm missing any):

            P1) Complete freedom for everybody. So any component can have whatever sub-namespace it wants.

            P2) Restrict it to be, always, APIs (event, comment, log, grade...). Of course new apis can be added there, and a possible candidate would be "addon" to let developers to have a complete sub-namespace available for them).

            P3) Reserve the APIs words, so nobody can use them but to implement the classes covering/implementing those APIs. Freedom to use everything else.

            So, that's basically about to decide between P1, P2 and P3 (and acknowledge that P0 exists).

            And for me (personal vote), the best option is P2 (with P3 being acceptable if everybody prefers it). And with P1 being a big "no, no!". Once more, my opinion, nothing else.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Eric, I agree contrib code should follow core rules. Just they are not enforced there, just recommended. And yes, contrib authors have a stake (and a steak) in this discussion. Nothing to object. Back to Q0002) I think there is already an agreement about level1 being always the component name. Nothing to discuss there. More yet, P0) Developers already own level1, so any class can be added there (namespaced or not namespaces, that's Q0001. There is complete freedom there (again with Q0001 to decide). Problem comes with level2, where there are these options (unless I'm missing any): P1) Complete freedom for everybody. So any component can have whatever sub-namespace it wants. P2) Restrict it to be, always, APIs (event, comment, log, grade...). Of course new apis can be added there, and a possible candidate would be "addon" to let developers to have a complete sub-namespace available for them). P3) Reserve the APIs words, so nobody can use them but to implement the classes covering/implementing those APIs. Freedom to use everything else. So, that's basically about to decide between P1, P2 and P3 (and acknowledge that P0 exists). And for me (personal vote), the best option is P2 (with P3 being acceptable if everybody prefers it). And with P1 being a big "no, no!". Once more, my opinion, nothing else. Ciao
            Hide
            emerrill Eric Merrill added a comment -

            I obviously agree with P0. I also agree that P1 is a no-no.

            My personal opinion is for P3 - I think it strikes the right balance between flexibility and control. I would create a blacklist of all the current API names (core, events, files, dml, form, etc.) - even those that do not use those namespaces at this time - and make those 'reserved'. Leave any other namespace open to the plugin dev. If there is a future API that causes a collision, so be it, the modules will just need to be fixed at that time.

            Show
            emerrill Eric Merrill added a comment - I obviously agree with P0. I also agree that P1 is a no-no. My personal opinion is for P3 - I think it strikes the right balance between flexibility and control. I would create a blacklist of all the current API names (core, events, files, dml, form, etc.) - even those that do not use those namespaces at this time - and make those 'reserved'. Leave any other namespace open to the plugin dev. If there is a future API that causes a collision, so be it, the modules will just need to be fixed at that time.
            Hide
            poltawski Dan Poltawski added a comment -

            (I'm ignoring your questions to me Eloy, because I think it might confuse the issue further, especially cos i'm confused by your questions).

            I am in favour of P3.

            Show
            poltawski Dan Poltawski added a comment - (I'm ignoring your questions to me Eloy, because I think it might confuse the issue further, especially cos i'm confused by your questions). I am in favour of P3.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm ignoring your questions to me Eloy...

            Well done. Surely my questions were more confusing than your proposal, lol.

            NVM, I'm getting all the information commented here and creating a final list for voting. Stay tuned, thanks everybody!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm ignoring your questions to me Eloy... Well done. Surely my questions were more confusing than your proposal, lol. NVM, I'm getting all the information commented here and creating a final list for voting. Stay tuned, thanks everybody!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Ok, I've tried to summarize everything here:

            http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces

            And have added sort of voting survey here:

            http://docs.moodle.org/dev/User_talk:Eloy_Lafuente_(stronk7)/Namespaces

            Feel free to comment any miss/mistake here. Don't pollute the summary with any comment (feel free to amend any typo and/or English offense, lol).

            Issue EOL: 36h from now (hopefully).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Ok, I've tried to summarize everything here: http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces And have added sort of voting survey here: http://docs.moodle.org/dev/User_talk:Eloy_Lafuente_(stronk7)/Namespaces Feel free to comment any miss/mistake here. Don't pollute the summary with any comment (feel free to amend any typo and/or English offense, lol). Issue EOL: 36h from now (hopefully). Ciao
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks Eloy, for creating the doc. Some comments:-
            U01 - why recommended? do we really want people to import all those common classes that is being used such as moodle_url() and context_* ?
            U05 - C), there is no need to be strict here imo.
            L02 - B), we surely can group classes properly by allowing this.
            A03 - C), Component restriction should be strictly followed, not so sure about the api restriction. Do we want all those sub directories with only one file ?
            S03 - c) (lol, isn't this a paradox, both always required and is forbidden? ) Anyways, +1 to use it only when needed, i.e when the class is not imported.

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks Eloy, for creating the doc. Some comments:- U01 - why recommended? do we really want people to import all those common classes that is being used such as moodle_url() and context_* ? U05 - C), there is no need to be strict here imo. L02 - B), we surely can group classes properly by allowing this. A03 - C), Component restriction should be strictly followed, not so sure about the api restriction. Do we want all those sub directories with only one file ? S03 - c) (lol, isn't this a paradox, both always required and is forbidden? ) Anyways, +1 to use it only when needed, i.e when the class is not imported. Thanks
            Hide
            quen Sam Marshall added a comment -

            I don't actually have too many opinions about this but am using namespaces in new development. I looked through the voting questions and honestly think most of them don't matter so if somebody picks an option it's probably OK.

            Just commenting because I think it's important to have namespacing required for new developments and included in coding style.

            Show
            quen Sam Marshall added a comment - I don't actually have too many opinions about this but am using namespaces in new development. I looked through the voting questions and honestly think most of them don't matter so if somebody picks an option it's probably OK. Just commenting because I think it's important to have namespacing required for new developments and included in coding style.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi Ankit,

            about U01, it's not for global scope functions but for heavily namespaced classes (in order to avoid \very\long\subspace\names\everywhere). In fact, S02 and S03 are the ones about global scope code.

            Also, I've moved your opinions to http://docs.moodle.org/dev/User_talk:Eloy_Lafuente_(stronk7)/Namespaces I've assumed your A03-C was more an "A" than a "B", not sure which "C" you were proposing. Ping me if needed.

            Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi Ankit, about U01, it's not for global scope functions but for heavily namespaced classes (in order to avoid \very\long\subspace\names\everywhere). In fact, S02 and S03 are the ones about global scope code. Also, I've moved your opinions to http://docs.moodle.org/dev/User_talk:Eloy_Lafuente_(stronk7)/Namespaces I've assumed your A03-C was more an "A" than a "B", not sure which "C" you were proposing. Ping me if needed. Thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            FYI, the Namespaces page above (and the survey/comments in its Talk page) have been locked. Time is over. Thanks everybody for your collaboration there!

            Now the integrators will meet in a dark room with some beer and cookies in order to discuss everything and get the definitive coding guidelines for namespaces ASAP.

            Again, thanks for your ideas and comments. And feel free to add anything new here.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - FYI, the Namespaces page above (and the survey/comments in its Talk page) have been locked. Time is over. Thanks everybody for your collaboration there! Now the integrators will meet in a dark room with some beer and cookies in order to discuss everything and get the definitive coding guidelines for namespaces ASAP. Again, thanks for your ideas and comments. And feel free to add anything new here. Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            Nooooooooooooooooo. I can't eat beer or cookies at the moment

            Show
            poltawski Dan Poltawski added a comment - Nooooooooooooooooo. I can't eat beer or cookies at the moment
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            NP, Zumba sessions then! LOL! Niao!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - NP, Zumba sessions then! LOL! Niao!
            Hide
            timhunt Tim Hunt added a comment -

            So, it seems this issue is a blocker for other stuff getting integrated. For example MDL-44251. So, updating priority to reflect that.

            Show
            timhunt Tim Hunt added a comment - So, it seems this issue is a blocker for other stuff getting integrated. For example MDL-44251 . So, updating priority to reflect that.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Hi, we have been thinking and discussing about this for some days, and just wanted to share with you about the point (L02) that is giving us the most headaches, so I'm pasting here the contents under debate.

            Ok,

            thanks for feedback. So it seems that there are 2 alternatives for the level2 thingy, ignoring any other from now:

            1) We forbid the use of any API as level2 (they get reserved for those API related things). We give 100% freedom for any other level2 word. Current code basically follows this.

            2) We create a new "core" level2 (or "api", why not, shorter and clear), where we put all those API classes (mainly events for now, but will grow, for sure). Obviously only APIs can go there and level2 gets 100% freedom for everything else. Obviously "api" will be the only level2 forbidden word. This requires to move current stuff but it a) does not require to be done now b) should be really easy to move. I bet any IDE can refactor that perfectly (99% of success).

            Finally, but not less important, I'd remark some points from my previous post:

            P1) namespaces are not mandatory.

            P2) global-scope code is not forbidden (yet).

            Also, I like the idea (Tim) of having a plan to move current datalib, weblib and friends to level2, "data", "web"... in the future, of course, and while not fitting into any API.

            Let's vote. As said, I've ignored other options coz think the 1) and 2) above are the only we can really implement (seems people is tremendously against to enclose their code inside a 3rd level ("componentapi" or similar), but loves/proposes to enclose all the core apis there. I really don't understand why, but ok, can accept it).

            ....
            ....

            Ciao

            PS: As commented in the initial post here, all the rest of rules and votes can be found at:

            (don't think there are other points to discuss there, but just in case you want to give all them a final review. The one we are deciding here and now is, exactly, L02)

            So basically 2 options (all the rest have been discarded):

            1) Allow both APIs (reserved) and developer's level2 to coexist. Can be problematic when new APIs land/are reorganized. That's more or less current way, so it son't require many changes.

            2) Move all APIs to an "api" level2 (that would be the only one reserved word) and bring developers the whole level2 space for their enjoyment. No conflicts because core apis will be confined/separated always. But requires to move all current (level2, \core\event* => \core\api\event and so on) code to that new directory (big but not complex to do at all).

            And that's the point we wanted to share here, any argument is welcome.

            Again, this is exclusively L02, plz avoid any other, unrelated comment. TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Hi, we have been thinking and discussing about this for some days, and just wanted to share with you about the point (L02) that is giving us the most headaches, so I'm pasting here the contents under debate. Ok, thanks for feedback. So it seems that there are 2 alternatives for the level2 thingy, ignoring any other from now: 1) We forbid the use of any API as level2 (they get reserved for those API related things). We give 100% freedom for any other level2 word. Current code basically follows this. 2) We create a new "core" level2 (or "api", why not, shorter and clear), where we put all those API classes (mainly events for now, but will grow, for sure). Obviously only APIs can go there and level2 gets 100% freedom for everything else. Obviously "api" will be the only level2 forbidden word. This requires to move current stuff but it a) does not require to be done now b) should be really easy to move. I bet any IDE can refactor that perfectly (99% of success). Finally, but not less important, I'd remark some points from my previous post: P1) namespaces are not mandatory. P2) global-scope code is not forbidden (yet). Also, I like the idea (Tim) of having a plan to move current datalib, weblib and friends to level2, "data", "web"... in the future, of course, and while not fitting into any API. Let's vote. As said, I've ignored other options coz think the 1) and 2) above are the only we can really implement (seems people is tremendously against to enclose their code inside a 3rd level ("componentapi" or similar), but loves/proposes to enclose all the core apis there. I really don't understand why, but ok, can accept it). .... .... Ciao PS: As commented in the initial post here, all the rest of rules and votes can be found at: A summary covering all the aspects commented exists @ http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces Some of the points there were open for voting / discuss @ http://docs.moodle.org/dev/User_talk:Eloy_Lafuente_(stronk7)/Namespaces (don't think there are other points to discuss there, but just in case you want to give all them a final review. The one we are deciding here and now is, exactly, L02) So basically 2 options (all the rest have been discarded): 1) Allow both APIs (reserved) and developer's level2 to coexist. Can be problematic when new APIs land/are reorganized. That's more or less current way, so it son't require many changes. 2) Move all APIs to an "api" level2 (that would be the only one reserved word) and bring developers the whole level2 space for their enjoyment. No conflicts because core apis will be confined/separated always. But requires to move all current (level2, \core\event* => \core\api\event and so on) code to that new directory (big but not complex to do at all). And that's the point we wanted to share here, any argument is welcome. Again, this is exclusively L02, plz avoid any other, unrelated comment. TIA and ciao
            Hide
            emerrill Eric Merrill added a comment -

            I will just add my fractional 2 cents.

            I would vote for #2. My pros:

            • Permanently de-conflicts core and component code.
            • Very easy to update existing code.
            • Nice and short.
            Show
            emerrill Eric Merrill added a comment - I will just add my fractional 2 cents. I would vote for #2. My pros: Permanently de-conflicts core and component code. Very easy to update existing code. Nice and short.
            Hide
            damyon Damyon Wiese added a comment -

            I vote #2 - but I like "core" better than API because it of consistency. ie \core\event\blah and \mod_biscuits\core\event\blah are part of the same API.

            Show
            damyon Damyon Wiese added a comment - I vote #2 - but I like "core" better than API because it of consistency. ie \core\event\blah and \mod_biscuits\core\event\blah are part of the same API.
            Hide
            marina Marina Glancy added a comment -

            My vote goes for the option suggested at the meeting:
            Plugins put their own classes in PLUGINDIR/classes/local/
            everything else is used to extend core apis, such as: events, renderers, etc.

            This looks the most reasonable because there are and will be much more core apis than plugin-local classes.

            Show
            marina Marina Glancy added a comment - My vote goes for the option suggested at the meeting: Plugins put their own classes in PLUGINDIR/classes/local/ everything else is used to extend core apis, such as: events, renderers, etc. This looks the most reasonable because there are and will be much more core apis than plugin-local classes.
            Hide
            skodak Petr Skoda added a comment -

            I personally do not see any major problems related to collisions. New core namespaces are going to be added in new dev branches only, it is technically possible to refactor you internal class namespace layout in plugin if there are any collisions in the future without breaking any compatibility outside of plugin.

            At the same time I agree we need some better rules and especially easy to find overview of second level namespaces that are expected by the core. My favourite option is 4 which was discussed today with a whitelisted second level namespaces that are reserved for plugins only - local, util, helper, etc.

            Sample proposed docs for option 4:

            Standard plugin namespaces with with strictly defined classes:

            • \mod_xx\classes\event\ - events triggered in plugin
            • \mod_xx\classes\task\ - cron task classes
            • \mod_xx\classes\plugininfo\ - subplugin descriptions
            • ...
              These are added only in major releases. There may be some blacklisted namespaces such as 'hook', 'api', 'renderer' which will be possibly used in future.

            Whitelisted plugin namespaces with freestyle structure:

            • local
            • util
              * helper
            • observer - recommended place for observers

            Anything other namespace not included in the whitelist may be claimed in the future which would require addon developers to refactor the code.

            Show
            skodak Petr Skoda added a comment - I personally do not see any major problems related to collisions. New core namespaces are going to be added in new dev branches only, it is technically possible to refactor you internal class namespace layout in plugin if there are any collisions in the future without breaking any compatibility outside of plugin. At the same time I agree we need some better rules and especially easy to find overview of second level namespaces that are expected by the core. My favourite option is 4 which was discussed today with a whitelisted second level namespaces that are reserved for plugins only - local, util, helper, etc. Sample proposed docs for option 4: Standard plugin namespaces with with strictly defined classes: \mod_xx\classes\event\ - events triggered in plugin \mod_xx\classes\task\ - cron task classes \mod_xx\classes\plugininfo\ - subplugin descriptions ... These are added only in major releases. There may be some blacklisted namespaces such as 'hook', 'api', 'renderer' which will be possibly used in future. Whitelisted plugin namespaces with freestyle structure: local util * helper observer - recommended place for observers Anything other namespace not included in the whitelist may be claimed in the future which would require addon developers to refactor the code.
            Hide
            quen Sam Marshall added a comment -

            Just to make this clearer with the \local thing - am I right in thinking that you do NOT need to add a pointless empty directory for this, because you can still have classes in the top-level namespace i.e. you can still have a class directly in mod/whatever/classes folder (in namespace mod_whatever), and it's only if you want multiple namespaces for your plugin because you're on crack or something that you need to make a directory /classes/local/custom/?

            If so then I don't have any objection to it.

            (On a tangent: A properly-implemented plugin using autoloading nowadays has not too much in its root directory, version.php and maybe lib.php, maybe styles.css, then nothing else, which is a bit of a waste. We should probably move all the files from the 'db' directory into component root, obviously having both places work, since half of them - services, events, access, ... - are nothing to do with the database. This is a totally separate issue but I wonder if it would be worth considering in the same release as recommending the namespaces thing, since that's already going to mean plugin developers ought to move files around.)

            Show
            quen Sam Marshall added a comment - Just to make this clearer with the \local thing - am I right in thinking that you do NOT need to add a pointless empty directory for this, because you can still have classes in the top-level namespace i.e. you can still have a class directly in mod/whatever/classes folder (in namespace mod_whatever), and it's only if you want multiple namespaces for your plugin because you're on crack or something that you need to make a directory /classes/local/custom/? If so then I don't have any objection to it. (On a tangent: A properly-implemented plugin using autoloading nowadays has not too much in its root directory, version.php and maybe lib.php, maybe styles.css, then nothing else, which is a bit of a waste. We should probably move all the files from the 'db' directory into component root, obviously having both places work, since half of them - services, events, access, ... - are nothing to do with the database. This is a totally separate issue but I wonder if it would be worth considering in the same release as recommending the namespaces thing, since that's already going to mean plugin developers ought to move files around.)
            Hide
            marina Marina Glancy added a comment -

            Sam, I'm not sure you understand it correctly. Plugin can not put something directly in /classes/ unless it implements core interface or extends core class with the same location (such as events, renderers, and [many] other apis in the future)

            Show
            marina Marina Glancy added a comment - Sam, I'm not sure you understand it correctly. Plugin can not put something directly in /classes/ unless it implements core interface or extends core class with the same location (such as events, renderers, and [many] other apis in the future)
            Hide
            damyon Damyon Wiese added a comment -

            Marina - I don't agree on that point. A plugin does not have to use a second level namespace for it's classes if it doesn't want to. There is no harm in this - there will never be collisions or namespace issues.

            See: mod/assign/feedback/editpdf/classes/

            Show
            damyon Damyon Wiese added a comment - Marina - I don't agree on that point. A plugin does not have to use a second level namespace for it's classes if it doesn't want to. There is no harm in this - there will never be collisions or namespace issues. See: mod/assign/feedback/editpdf/classes/
            Hide
            marina Marina Glancy added a comment -

            In this case I did not understand it correctly (and still don't). I'll just wait for decision

            Show
            marina Marina Glancy added a comment - In this case I did not understand it correctly (and still don't). I'll just wait for decision
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Just to clarify things (coz I think we are mixing things):

            0) level 0, aka non-namespaced classes under "classes" are 100% allowed for developers. That applies to any component (core, plugins, subsystems and addons (aka, contrib). It just works by calling those classes/interfaces/traits: "component_xxxxx" and being stored under classes/xxxxx.php. These are not issue. Examples:

            • core: lib/classes/text.php => core_text
            • subsystem: course/classes/deletecategory_form.php => core_course_deletecategory_form
            • plugin/addon: mod/assign/feedback/editpdf/classes/renderer.php => assignfeedback_editpdf_renderer

            1) level 1, namespaced classes under "classes" are also 100% allowed for developers. That applies to any component (core, plugins, subsystems and addons). It just works by calling (namespacing) those classes/interfaces/traits with "\component\xxxxx" and being stored under classes/xxxxx.php. These are not issue either. Examples:

            • core: apparently there is none in core component right now.
            • subsystem: calendar/classes/type_base.php => \core_calendar\type_base
            • plugin/addon: mod/assign/feedback/editpdf/classes/annotation.php => \assignfeedback_editpdf\annotation

            Those 2 cases are not problematic and developer owns them completely. Note that's true for ANY component (core, subsystem, plugin, addon). Total freedom there (of course some order, consistency is always recommended, avoiding non-sense mixes and so on).

            2) level 2 is what we are discussing about. That and nothing else. Coming soon with what I've found to be the final proposal once heard all the POVs.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Just to clarify things (coz I think we are mixing things): 0) level 0, aka non-namespaced classes under "classes" are 100% allowed for developers. That applies to any component (core, plugins, subsystems and addons (aka, contrib). It just works by calling those classes/interfaces/traits: "component_xxxxx" and being stored under classes/xxxxx.php. These are not issue. Examples: core: lib/classes/text.php => core_text subsystem: course/classes/deletecategory_form.php => core_course_deletecategory_form plugin/addon: mod/assign/feedback/editpdf/classes/renderer.php => assignfeedback_editpdf_renderer 1) level 1, namespaced classes under "classes" are also 100% allowed for developers. That applies to any component (core, plugins, subsystems and addons). It just works by calling (namespacing) those classes/interfaces/traits with "\component\xxxxx" and being stored under classes/xxxxx.php. These are not issue either. Examples: core: apparently there is none in core component right now. subsystem: calendar/classes/type_base.php => \core_calendar\type_base plugin/addon: mod/assign/feedback/editpdf/classes/annotation.php => \assignfeedback_editpdf\annotation Those 2 cases are not problematic and developer owns them completely. Note that's true for ANY component (core, subsystem, plugin, addon). Total freedom there (of course some order, consistency is always recommended, avoiding non-sense mixes and so on). 2) level 2 is what we are discussing about. That and nothing else. Coming soon with what I've found to be the final proposal once heard all the POVs. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Proposal for level 2 (for level 0 and 1, see the previous comment):

            Simply and raw:

            Any component (again, core, subsystem, plugin, addon) will be able to use any of these level2 namespaces in their classes/interfaces/traits:

            • API names. http://docs.moodle.org/dev/Core_APIs
            • "local": For stuff related with the component business.
            • "helper": For instantiable stuff that may be related with component business.
            • "util": For, usually static, stuff candidate to promote to core.

            Obviously that implies:

            1) "local", "helper" and "util" won't be ever APIs.

            2) anything in level2 not being an API and not being one of those 3 whitelisted exceptions is INVALID.

            3) Side note, because there is some ongoing stuff about this, the previous point means that, explicitly, \core\observer\xxxx are INVALID. Move from there to any of the valid level0, level1 or level2 alternatives. If you ask me, my opinion is that those ones are part of the "event" API, can create a level3 "observer" there. Or move them to any other valid place.

            Please, consider it, I think it sums everybody wises while keep things under control. May require to move some current stuff, but hey, that should be trivial! Some +-1 will be welcome to the proposal.

            Edited: Changed "observers" to "observer", trying to keep levels always singular.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Proposal for level 2 (for level 0 and 1, see the previous comment): Simply and raw: Any component (again, core, subsystem, plugin, addon) will be able to use any of these level2 namespaces in their classes/interfaces/traits: API names. http://docs.moodle.org/dev/Core_APIs "local": For stuff related with the component business. "helper": For instantiable stuff that may be related with component business. "util": For, usually static, stuff candidate to promote to core. Obviously that implies: 1) "local", "helper" and "util" won't be ever APIs. 2) anything in level2 not being an API and not being one of those 3 whitelisted exceptions is INVALID. 3) Side note, because there is some ongoing stuff about this, the previous point means that, explicitly, \core\observer\xxxx are INVALID. Move from there to any of the valid level0, level1 or level2 alternatives. If you ask me, my opinion is that those ones are part of the "event" API, can create a level3 "observer" there. Or move them to any other valid place. Please, consider it, I think it sums everybody wises while keep things under control. May require to move some current stuff, but hey, that should be trivial! Some +-1 will be welcome to the proposal. Edited: Changed "observers" to "observer", trying to keep levels always singular.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Finally, tangentially, I wanted to add here 2 little rules, easy to fullfill, that were not originally in the list of specifications:

            1) namespaces may have a phpdoc block.
            2) always put a blank line after the namespace declaration and after the last "use" declaration. Readability.

            Please, avoid discussing about these two, they don't owe that. I've added them to:

            http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces#Some_extra_tiny_details

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Finally, tangentially, I wanted to add here 2 little rules, easy to fullfill, that were not originally in the list of specifications: 1) namespaces may have a phpdoc block. 2) always put a blank line after the namespace declaration and after the last "use" declaration. Readability. Please, avoid discussing about these two, they don't owe that. I've added them to: http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces#Some_extra_tiny_details Ciao
            Hide
            quen Sam Marshall added a comment -

            Eloy: +1

            Show
            quen Sam Marshall added a comment - Eloy: +1
            Hide
            mudrd8mz David Mudrak added a comment -

            Sounds like a good compromise (as usually, compromise is a solution that does not really fit anybody :-p). Seriously, good work Eloy on summarizing this. I read it that there are (as it happens in Moodle dev world) multiple ways how to organise things. Let's see how they evolve (reminds me of renderers' public versus protected render_* methods).

            Show
            mudrd8mz David Mudrak added a comment - Sounds like a good compromise (as usually, compromise is a solution that does not really fit anybody :-p). Seriously, good work Eloy on summarizing this. I read it that there are (as it happens in Moodle dev world) multiple ways how to organise things. Let's see how they evolve (reminds me of renderers' public versus protected render_* methods).
            Hide
            timhunt Tim Hunt added a comment -

            I see no point in allowing both helper and util. The difference is not clear to me, and not clear just from the names. Even if we choose to make both reserved words (never call a core API that) we should only allow the use of one.

            I don't really care which, but if I had to vote, I would go for util becuase it is shorter.

            Show
            timhunt Tim Hunt added a comment - I see no point in allowing both helper and util. The difference is not clear to me, and not clear just from the names. Even if we choose to make both reserved words (never call a core API that) we should only allow the use of one. I don't really care which, but if I had to vote, I would go for util becuase it is shorter.
            Hide
            poltawski Dan Poltawski added a comment -

            I see no point in allowing both helper and util. [...] if I had to vote, I would go for util becuase it is shorter.

            +1.

            Show
            poltawski Dan Poltawski added a comment - I see no point in allowing both helper and util. [...] if I had to vote, I would go for util becuase it is shorter. +1.
            Hide
            timhunt Tim Hunt added a comment -

            In fact, the discussion in dev chat (https://moodle.org/local/chatlogs/index.php?conversationid=14962#c530913) went on to suggest that acutally we should just have local (and API names), which is what gets my vote at the moment.

            Show
            timhunt Tim Hunt added a comment - In fact, the discussion in dev chat ( https://moodle.org/local/chatlogs/index.php?conversationid=14962#c530913 ) went on to suggest that acutally we should just have local (and API names), which is what gets my vote at the moment.
            Hide
            poltawski Dan Poltawski added a comment -

            Argh, no, not more options noooooooooooooo

            Show
            poltawski Dan Poltawski added a comment - Argh, no, not more options noooooooooooooo
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            (friday's joking: "help" is same length, although there are more distance between keys, uhm...should we create a subtask for that). ROFL, could not resist, sorry.

            I don't mind, really, I just "opened the hands" to accept "helper" and "util" if that would make more people happy. So, only "local", "local&util" or "local&util&helper" are good for me. That and official APIs, of course.

            Let's wait till Tuesday to people to object anything else and express their opinion about the local-util-helper detail.

            Then I'll decide, said. Ciao (and thanks)

            Edited: ROFLing.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited (friday's joking: "help" is same length, although there are more distance between keys, uhm...should we create a subtask for that). ROFL, could not resist, sorry. I don't mind, really, I just "opened the hands" to accept "helper" and "util" if that would make more people happy. So, only "local", "local&util" or "local&util&helper" are good for me. That and official APIs, of course. Let's wait till Tuesday to people to object anything else and express their opinion about the local-util-helper detail. Then I'll decide, said. Ciao (and thanks) Edited: ROFLing.
            Hide
            damyon Damyon Wiese added a comment -

            Sounds all good and sensible to me Eloy. I would vote for "local" and not "util" or "helper" - but no matter if they are included.

            Show
            damyon Damyon Wiese added a comment - Sounds all good and sensible to me Eloy. I would vote for "local" and not "util" or "helper" - but no matter if they are included.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            OK, sold. Only "APIs" + "local" will be allowed. Finito!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - OK, sold. Only "APIs" + "local" will be allowed. Finito!
            Hide
            mudrd8mz David Mudrak added a comment -

            Yay!

            Show
            mudrd8mz David Mudrak added a comment - Yay!
            Hide
            timhunt Tim Hunt added a comment -

            Bikeshed: Painted!

            And what a lovely orange colour it is. Thanks all.

            Show
            timhunt Tim Hunt added a comment - Bikeshed: Painted! And what a lovely orange colour it is. Thanks all.
            Hide
            mudrd8mz David Mudrak added a comment -

            I would like to get this documented. Eloy Lafuente (stronk7), can we consider this approved and closed by now?

            Show
            mudrd8mz David Mudrak added a comment - I would like to get this documented. Eloy Lafuente (stronk7) , can we consider this approved and closed by now?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Oh, David, it would be awesome if you can get some time to convert the the agreed rules into something readable. I've started the task a number of times, but always found excuses to delay it (grrr).

            I've just amended http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces marking all the the decisions taken after voting/discussing here and in forums.

            So, really, if you can take your abilities over this... it would be awesome. Feel free to get this assigned if that helps.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Oh, David, it would be awesome if you can get some time to convert the the agreed rules into something readable. I've started the task a number of times, but always found excuses to delay it (grrr). I've just amended http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces marking all the the decisions taken after voting/discussing here and in forums. So, really, if you can take your abilities over this... it would be awesome. Feel free to get this assigned if that helps. Ciao
            Show
            poltawski Dan Poltawski added a comment - I just added added: http://docs.moodle.org/dev/index.php?title=Coding_style&diff=45645&oldid=45620 Following: MDLSITE-3224
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Yay, great!

            I also have added a note to:

            http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces

            So closing this, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Yay, great! I also have added a note to: http://docs.moodle.org/dev/User:Eloy_Lafuente_(stronk7)/Namespaces So closing this, thanks!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                11 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development