Moodle
  1. Moodle
  2. MDL-21249

Decide once and for all on how to use phpdoc @package and @subpackage

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      7098

      Description

      THIS WAS INITIAL PROPOSAL AND IT HAS CHANGED LATER - READ THE COMMENTS

      This is a proposal to use standard Moodle component name as @package identifier in a file or class documentation block. Examples:

      @package core
      @package core_groups
      @package mod_glossary
      @package mod_workshop
      @package workshopform_accumulative
      @package workshopeval_best
      @package question
      @package auth_ldap

      etc.

        Issue Links

          Activity

          Hide
          David Mudrak added a comment -

          ping pong

          Please comment and vote as will

          Show
          David Mudrak added a comment - ping pong Please comment and vote as will
          Hide
          Petr Škoda added a comment - - edited

          alternative is to use:

          @package core
          @subpackage group
          ------
          @package mod
          @subpackage forum
          ------
          @package workshopform
          @subpackage accumulative

          Show
          Petr Škoda added a comment - - edited alternative is to use: @package core @subpackage group ------ @package mod @subpackage forum ------ @package workshopform @subpackage accumulative
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ho,

          I think that, really, the key point here is how is that @subpackage information showed by phpdocumentor and friends.

          If it produces some sort of navigable grouping, so only (a few) @packages are showed initially and later you see the @subpackages, then I prefer Petr's alternative (we only have to agree about the top-level packages to use). That's better than showing hundreds of packages IMO (David's alternative).

          If the @subpackages aren't grouped by phpdocumentor in order to provide easier navigation... then really it doesn't matter too much.

          In fact it's a pity not being able to use dots to provide "paths" like "mod.forum.backup" as @package, with each part of the path being "clickable" to the corresponding part of the documentation (much like Javadoc, I think).

          Finally, note that I don't buy the final example from Petr at all:

          @package workshopform
          @subpackage accumulative

          nor David's alternative either, obviously:

          @package workshopform_accumulative

          IMO workshopform cannot be a top level package in any case. It should be something like:

          @package mod
          @subpackage workshop_grading_accumulative
          (In Petr's way)

          or:

          @package mod_workshop_grading_accumulative
          (In David's way)

          But we must keep the root levels well defined for easier navigation and grouping.

          That's all... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Ho, I think that, really, the key point here is how is that @subpackage information showed by phpdocumentor and friends. If it produces some sort of navigable grouping, so only (a few) @packages are showed initially and later you see the @subpackages, then I prefer Petr's alternative (we only have to agree about the top-level packages to use). That's better than showing hundreds of packages IMO (David's alternative). If the @subpackages aren't grouped by phpdocumentor in order to provide easier navigation... then really it doesn't matter too much. In fact it's a pity not being able to use dots to provide "paths" like "mod.forum.backup" as @package, with each part of the path being "clickable" to the corresponding part of the documentation (much like Javadoc, I think). Finally, note that I don't buy the final example from Petr at all: @package workshopform @subpackage accumulative nor David's alternative either, obviously: @package workshopform_accumulative IMO workshopform cannot be a top level package in any case. It should be something like: @package mod @subpackage workshop_grading_accumulative (In Petr's way) or: @package mod_workshop_grading_accumulative (In David's way) But we must keep the root levels well defined for easier navigation and grouping. That's all... ciao
          Hide
          David Mudrak added a comment -

          My reasoning was that once workshopform is a legal top-level component type in Moodle, it should be mapped to a @package. And it really is - I can call get_string('thissucks', 'workshopform_accumulative'). But I am OK with

          @package mod_workshop
          @subpackage workshopform_accumulative

          to make it clear that workshopform can't be used without mod_workshop

          Show
          David Mudrak added a comment - My reasoning was that once workshopform is a legal top-level component type in Moodle, it should be mapped to a @package. And it really is - I can call get_string('thissucks', 'workshopform_accumulative'). But I am OK with @package mod_workshop @subpackage workshopform_accumulative to make it clear that workshopform can't be used without mod_workshop
          Hide
          Penny Leach added a comment -

          I think

          @package mod_workshop
          @subpackage form_accumulative

          makes most sense to me - would that result in incorrect nav later though?

          and I don't like repeating workshop in both like in David's comment above

          Show
          Penny Leach added a comment - I think @package mod_workshop @subpackage form_accumulative makes most sense to me - would that result in incorrect nav later though? and I don't like repeating workshop in both like in David's comment above
          Hide
          Petr Škoda added a comment -

          hmm, @package mod_workshop @subpackage form_accumulative
          is inconsistent with get_string() and anything else in the plugin infrastructure, my -1 because it seems to be confusing

          module subplugins are really named:
          @package workshopform (plugin type)
          @subpackage accumulative (plugin name)

          I know it is ugly, but the "subplugins" idea itself is not very nice and forced us to create this "weird" naming conventions

          Show
          Petr Škoda added a comment - hmm, @package mod_workshop @subpackage form_accumulative is inconsistent with get_string() and anything else in the plugin infrastructure, my -1 because it seems to be confusing module subplugins are really named: @package workshopform (plugin type) @subpackage accumulative (plugin name) I know it is ugly, but the "subplugins" idea itself is not very nice and forced us to create this "weird" naming conventions
          Hide
          David Mudrak added a comment -

          Agree with Petr. the point of this proposal is to be consistent with how the components are used in the code - in get_string(), function name prefixes, renderer names etc.

          Show
          David Mudrak added a comment - Agree with Petr. the point of this proposal is to be consistent with how the components are used in the code - in get_string(), function name prefixes, renderer names etc.
          Hide
          David Mudrak added a comment -

          Things have evolved a bit and frankenstyle pluginstyle_pluginname is used more and more in Moodle. It does not matter what we feel would be the best way. What matters is how phpdoc processors deal with @tags. And if you look at http://phpdocs.moodle.org/ it is clear that my original proposal is not useable and we should go Petr's way. @packages are displayed in the top frame and @subpackages are used as "chapter" names in the left frame.

          So, I am changing the proposal in the description of this issue to: let us consistently use

          @package    plugintype
          @subpackage pluginname
          

          Then the top frame will contain the list of areas in your code like core, mod, auth, enrol, ... and the files, classes and functions in left frame will be categorized as (eg when selecting mod) assignment, choice, data, ...

          Yes yes this is not crucial for 2.0 and of course we all have more important things to do. But I face this everytime I create new php file or fix missing phpdoc statement. Please vote at will

          Show
          David Mudrak added a comment - Things have evolved a bit and frankenstyle pluginstyle_pluginname is used more and more in Moodle. It does not matter what we feel would be the best way. What matters is how phpdoc processors deal with @tags. And if you look at http://phpdocs.moodle.org/ it is clear that my original proposal is not useable and we should go Petr's way. @packages are displayed in the top frame and @subpackages are used as "chapter" names in the left frame. So, I am changing the proposal in the description of this issue to: let us consistently use @ package plugintype @subpackage pluginname Then the top frame will contain the list of areas in your code like core, mod, auth, enrol, ... and the files, classes and functions in left frame will be categorized as (eg when selecting mod) assignment, choice, data, ... Yes yes this is not crucial for 2.0 and of course we all have more important things to do. But I face this everytime I create new php file or fix missing phpdoc statement. Please vote at will
          Hide
          Tim Hunt added a comment -

          I vote for following the current coding guidelines:

          @package pluginname

          or

          @package core
          @subpackage something

          Show
          Tim Hunt added a comment - I vote for following the current coding guidelines: @package pluginname or @package core @subpackage something
          Hide
          David Mudrak added a comment -

          The problem I see with the current style (see Tim's comment above) is that we end with dozens of pluginnames which makes the navigation over packages in phpdocumentator a bit useless. If we go "@package plugintype" way, the navigation structure itself makes clear overview of what plugin types Moodle consists of.

          Show
          David Mudrak added a comment - The problem I see with the current style (see Tim's comment above) is that we end with dozens of pluginnames which makes the navigation over packages in phpdocumentator a bit useless. If we go "@package plugintype" way, the navigation structure itself makes clear overview of what plugin types Moodle consists of.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ping to Petr:

          grep -lr "@package.*code" *

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Ping to Petr: grep -lr "@package.*code" * Ciao
          Hide
          Petr Škoda added a comment -

          GRRRR

          Show
          Petr Škoda added a comment - GRRRR
          Hide
          Jay Knight added a comment -

          It appears that one of these commits broke the installer, see MDL-23528.

          Show
          Jay Knight added a comment - It appears that one of these commits broke the installer, see MDL-23528 .
          Hide
          Petr Škoda added a comment -

          oops, fixing installer

          Show
          Petr Škoda added a comment - oops, fixing installer
          Hide
          Petr Škoda added a comment -

          should be fixed, thanks for the report!!

          Show
          Petr Škoda added a comment - should be fixed, thanks for the report!!
          Hide
          Akin Delamarre added a comment -

          Any word on which format contrib developers should use?

          Show
          Akin Delamarre added a comment - Any word on which format contrib developers should use?
          Hide
          Martin Dougiamas added a comment -

          +100 for the original proposal

          Show
          Martin Dougiamas added a comment - +100 for the original proposal
          Hide
          Tim Hunt added a comment -

          Martin, if you are going to say that, you should probably edit the bug summary.

          Show
          Tim Hunt added a comment - Martin, if you are going to say that, you should probably edit the bug summary.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Oki, here it is my last proposal, I commented @ HQ chat that I was going to put it here, so I'm doing. Feel free to ignore it, modify it, blame it or whatever.

          Premises
          ========

          1. We have these pieces of information:
            • core: the word, to denote anything belonging to core
            • plugintype: mod/block/report/qtype...
            • pluginname: each one of the implementations of a given plugintype.
            • subsystem: some parts of code that can be used by core (only once) or by plugins (in a transversal way): backup, completion, rss, tags, comments...
            • subplugintype: exclusively within mod plugins, one more level of arbitrary subdivision (workshopform...)
            • subpluginname: each one of the implementations of a given subplugintype.
          2. component: We are using, each time more, the "components" defined as: "plugintype_pluginname" or "core_subsystem", so we should use them as much as possible.
          3. subcomponent: Defined as "subplugintype_subpluginname" (just invented it, not sure if normalize_component() supports it or no.

          Proposal
          ========
          Using the concepts above:

          @package component|core
          @subpackage subcomponent|subsystem
          

          Rules
          =====
          Just 2, to observe always:

          1. package will be always component (or core if component does not apply).
          2. subpackage when applicable, will be always, subcomponent or subsystem (all subs together), with subcomponent taking precedence over subsystem.

          Examples
          ========

          Core stuff, like, say, moodlelib:
              @package core
          
          Backup core: (NT1*)
              @package core_backup
              @subpackage backup
          
          Workshop plugin:
              @package mod_workshop
          
          Workshop backup:
              @package mod_workshop
              @subpackage backup   
          
          Workshop workshopform rubric:
              @package mod_workshop
              @subpackage workshopform_rubric 
          
          Backup of workshopform rubric: (NT2*)
              @package mod_workshop
              @subpackage workshopform_rubric 
          

          Comments
          ========

          • NT1 = yes, it's a bit repetitive but can be useful for groupings by package or subpackage. And it's 100% observing the proposal rules.
          • NT2 = yes, we miss the subsystem information in subplugins, but is that really important? Also observes the proposal rules 100%.

          End
          ===
          And that is all. I think it's simple and clear. components everywhere and subs go to subpackages always. No exceptions, just follow the rules.

          Take it, blame it or throw it. I'not going to spend 1-more-second thinking about this and will accept whatever is finally decided.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Oki, here it is my last proposal, I commented @ HQ chat that I was going to put it here, so I'm doing. Feel free to ignore it, modify it, blame it or whatever. Premises ======== We have these pieces of information: core : the word, to denote anything belonging to core plugintype : mod/block/report/qtype... pluginname : each one of the implementations of a given plugintype. subsystem : some parts of code that can be used by core (only once) or by plugins (in a transversal way): backup, completion, rss, tags, comments... subplugintype : exclusively within mod plugins, one more level of arbitrary subdivision (workshopform...) subpluginname : each one of the implementations of a given subplugintype. component : We are using, each time more, the "components" defined as: "plugintype_pluginname" or "core_subsystem", so we should use them as much as possible. subcomponent : Defined as "subplugintype_subpluginname" (just invented it, not sure if normalize_component() supports it or no. Proposal ======== Using the concepts above: @ package component|core @subpackage subcomponent|subsystem Rules ===== Just 2, to observe always: package will be always component (or core if component does not apply). subpackage when applicable, will be always, subcomponent or subsystem (all subs together), with subcomponent taking precedence over subsystem. Examples ======== Core stuff, like, say, moodlelib: @ package core Backup core: (NT1*) @ package core_backup @subpackage backup Workshop plugin: @ package mod_workshop Workshop backup: @ package mod_workshop @subpackage backup Workshop workshopform rubric: @ package mod_workshop @subpackage workshopform_rubric Backup of workshopform rubric: (NT2*) @ package mod_workshop @subpackage workshopform_rubric Comments ======== NT1 = yes, it's a bit repetitive but can be useful for groupings by package or subpackage. And it's 100% observing the proposal rules. NT2 = yes, we miss the subsystem information in subplugins, but is that really important? Also observes the proposal rules 100%. End === And that is all. I think it's simple and clear. components everywhere and subs go to subpackages always. No exceptions, just follow the rules. Take it, blame it or throw it. I'not going to spend 1-more-second thinking about this and will accept whatever is finally decided. Ciao
          Hide
          Martin Dougiamas added a comment -

          Sorry Eloy, but that's not super clear to me, actually, and looks subject to being done wrongly by other developers.

          It should be:

          @package: (required) The full frankenstyle name of the current module, or "core" if there isn't one.
          @subpackage: (optional) The name of the API it belongs to, from a defined list of APIs. eg "backup", "renderer", "comment", "rating" etc

          Show
          Martin Dougiamas added a comment - Sorry Eloy, but that's not super clear to me, actually, and looks subject to being done wrongly by other developers. It should be: @package: (required) The full frankenstyle name of the current module, or "core" if there isn't one. @subpackage: (optional) The name of the API it belongs to, from a defined list of APIs. eg "backup", "renderer", "comment", "rating" etc
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          1) "full frankenstyle name" = component ? Note that components are BOTH plugins and subsystems. aka, my proposal above for @package.

          2) API = subsystem ? Note it is also my proposal above (for @subpackage), just I included support for module subplugins while yours ignore them completely (or puts them at the same level than the plugins).

          Anyway, as said above, np. I'd just suggest to explicitly put the examples above under your rules, just to understand 100% and being able to start enforcing them from now.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited 1) "full frankenstyle name" = component ? Note that components are BOTH plugins and subsystems. aka, my proposal above for @package. 2) API = subsystem ? Note it is also my proposal above (for @subpackage), just I included support for module subplugins while yours ignore them completely (or puts them at the same level than the plugins). Anyway, as said above, np. I'd just suggest to explicitly put the examples above under your rules, just to understand 100% and being able to start enforcing them from now. Ciao
          Hide
          Martin Dougiamas added a comment - - edited

          Yep, sure. The list of APIs (with definitive short names to use) is here: http://docs.moodle.org/dev/Core_APIs

          Note that this is a SUBSET of what we normally call "subsystems" (as defined by get_core_subsystems()) and is purely for conceptual understanding and documentation purposes. So I would call always refer to this list as "Core APIs".

          Just think of subpackage as a "sub-package of Moodle core" rather than a "sub-package of the @package".

          And yes, I want to keep all the plugins at one level. If we want to display them in any hierarchy we can program that logic in any external display of the phpdocs, in just the same way as we did for http://moodle.org/plugins

          So for the files in your examples:

          Core stuff, like, say, moodlelib:
              @package core
          
          Backup core: (NT1*)
              @package core_backup
              @subpackage backup
          
          Workshop plugin:
              @package mod_workshop
          
          Workshop backup:
              @package mod_workshop
              @subpackage backup   
          
          Workshop workshopform rubric:
              @package workshopform_rubric
          
          Backup of workshopform rubric: 
              @package workshopform_rubric
              @subpackage backup
          
          Show
          Martin Dougiamas added a comment - - edited Yep, sure. The list of APIs (with definitive short names to use) is here: http://docs.moodle.org/dev/Core_APIs Note that this is a SUBSET of what we normally call "subsystems" (as defined by get_core_subsystems()) and is purely for conceptual understanding and documentation purposes. So I would call always refer to this list as "Core APIs". Just think of subpackage as a "sub-package of Moodle core" rather than a "sub-package of the @package". And yes, I want to keep all the plugins at one level. If we want to display them in any hierarchy we can program that logic in any external display of the phpdocs, in just the same way as we did for http://moodle.org/plugins So for the files in your examples: Core stuff, like, say, moodlelib: @ package core Backup core: (NT1*) @ package core_backup @subpackage backup Workshop plugin: @ package mod_workshop Workshop backup: @ package mod_workshop @subpackage backup Workshop workshopform rubric: @ package workshopform_rubric Backup of workshopform rubric: @ package workshopform_rubric @subpackage backup
          Hide
          David Mudrak added a comment -

          OK. Let me have the last proposal re this issue. I like what Martin suggests. I would just prefer using @category tag instead of the @subpackage if it is supposed to describe the relevant API. Then we could even have subplugins being part of their parents package as they really are.

          See http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.category.pkg.html

          The examples would then look like:

          Core stuff, like, say, moodlelib:
              @package core
          
          Backup core:
              @package core_backup
              @category backup
          
          Workshop plugin:
              @package mod_workshop
          
          Workshop backup:
              @package mod_workshop
              @category backup   
          
          Workshop workshopform rubric:
              @package mod_workshop
              @subpackage workshopform_rubric
          
          Backup of workshopform rubric: 
              @package mod_workshop
              @subpackage workshopform_rubric
              @category backup
          

          This syntax would make me happy as it contains all information needed: Core API reference, frankentyle and subplugins dependency. The subplugins dependency is what I miss in Martin's examples.

          Show
          David Mudrak added a comment - OK. Let me have the last proposal re this issue. I like what Martin suggests. I would just prefer using @category tag instead of the @subpackage if it is supposed to describe the relevant API. Then we could even have subplugins being part of their parents package as they really are. See http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.category.pkg.html The examples would then look like: Core stuff, like, say, moodlelib: @ package core Backup core: @ package core_backup @category backup Workshop plugin: @ package mod_workshop Workshop backup: @ package mod_workshop @category backup Workshop workshopform rubric: @ package mod_workshop @subpackage workshopform_rubric Backup of workshopform rubric: @ package mod_workshop @subpackage workshopform_rubric @category backup This syntax would make me happy as it contains all information needed: Core API reference, frankentyle and subplugins dependency. The subplugins dependency is what I miss in Martin's examples.
          Hide
          David Mudrak added a comment -

          What I like most is that @subpackage is used for our subplugins which makes perfect sense imho. @category is used for grouping code per referenced core API.

          Show
          David Mudrak added a comment - What I like most is that @subpackage is used for our subplugins which makes perfect sense imho. @category is used for grouping code per referenced core API.
          Hide
          Petr Škoda added a comment -

          +1 for David's proposal

          Show
          Petr Škoda added a comment - +1 for David's proposal
          Hide
          Martin Dougiamas added a comment -

          Ah dammit, why did no-one notice @category before? :/

          http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.category.pkg.html

          That does look to be a better thing for grouping things (even it says it's for grouping packages). I don't know if many tools support it though.

          It's a bit late now .... We'd have to get everyone re-edit their patches.

          However I still think it's better to ALWAYS use the full plugin name like:

          @package workshopform_rubric

          Otherwise all these exceptions for subplugins get rather complicated. We have to explain different rules to people working on different plugins in http://moodle.org/plugins . And it's already hard enough getting them to do phpdocs consistently. Even senior core developers for that matter.

          Show
          Martin Dougiamas added a comment - Ah dammit, why did no-one notice @category before? :/ http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.category.pkg.html That does look to be a better thing for grouping things (even it says it's for grouping packages). I don't know if many tools support it though. It's a bit late now .... We'd have to get everyone re-edit their patches. However I still think it's better to ALWAYS use the full plugin name like: @package workshopform_rubric Otherwise all these exceptions for subplugins get rather complicated. We have to explain different rules to people working on different plugins in http://moodle.org/plugins . And it's already hard enough getting them to do phpdocs consistently. Even senior core developers for that matter.
          Hide
          David Mudrak added a comment -

          Is it really that late? If we accept it as the policy for the new patches, I volunteer myself to grep for @subpackage usage and change it to @category

          Show
          David Mudrak added a comment - Is it really that late? If we accept it as the policy for the new patches, I volunteer myself to grep for @subpackage usage and change it to @category
          Hide
          Aparup Banerjee added a comment -

          i like @category!

          Show
          Aparup Banerjee added a comment - i like @category!
          Hide
          Martin Dougiamas added a comment -

          We went with @category and everything is going to use that. To be explicit,

          1) We use @package to mention the full component name everywhere we can (currently focussing on API files but we'll have to eventually extend this everywhere).

          2) We use @category to highlight classes, files and functions that are important documentation for our APIs.

          If anyone wants to use @subpackage for some local partitioning within a component they are welcome to, it's not being used otherwise.

          Show
          Martin Dougiamas added a comment - We went with @category and everything is going to use that. To be explicit, 1) We use @package to mention the full component name everywhere we can (currently focussing on API files but we'll have to eventually extend this everywhere). 2) We use @category to highlight classes, files and functions that are important documentation for our APIs. If anyone wants to use @subpackage for some local partitioning within a component they are welcome to, it's not being used otherwise.
          Hide
          Martin Dougiamas added a comment -

          This now closed. Thanks all.

          See MDL-30971 for initial work in converting to this.

          Show
          Martin Dougiamas added a comment - This now closed. Thanks all. See MDL-30971 for initial work in converting to this.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: