Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-21249

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

    Details

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

      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.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              mudrd8mz David Mudrák added a comment -

              ping pong

              Please comment and vote as will

              Show
              mudrd8mz David Mudrák added a comment - ping pong Please comment and vote as will
              Hide
              skodak Petr Skoda added a comment - - edited

              alternative is to use:

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

              Show
              skodak Petr Skoda added a comment - - edited alternative is to use: @package core @subpackage group ------ @package mod @subpackage forum ------ @package workshopform @subpackage accumulative
              Hide
              stronk7 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
              stronk7 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              mjollnir 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
              mjollnir 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
              skodak Petr Skoda 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
              skodak Petr Skoda 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              timhunt Tim Hunt added a comment -

              I vote for following the current coding guidelines:

              @package pluginname

              or

              @package core
              @subpackage something

              Show
              timhunt Tim Hunt added a comment - I vote for following the current coding guidelines: @package pluginname or @package core @subpackage something
              Hide
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Ping to Petr:

              grep -lr "@package.*code" *

              Ciao

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

              GRRRR

              Show
              skodak Petr Skoda added a comment - GRRRR
              Hide
              jk3us Jay Knight added a comment -

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

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

              oops, fixing installer

              Show
              skodak Petr Skoda added a comment - oops, fixing installer
              Hide
              skodak Petr Skoda added a comment -

              should be fixed, thanks for the report!!

              Show
              skodak Petr Skoda added a comment - should be fixed, thanks for the report!!
              Hide
              adelamarre Akin Delamarre added a comment -

              Any word on which format contrib developers should use?

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

              +100 for the original proposal

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

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

              Show
              timhunt Tim Hunt added a comment - Martin, if you are going to say that, you should probably edit the bug summary.
              Hide
              stronk7 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
              stronk7 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
              dougiamas 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
              dougiamas 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
              stronk7 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
              stronk7 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
              dougiamas 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
              dougiamas 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              skodak Petr Skoda added a comment -

              +1 for David's proposal

              Show
              skodak Petr Skoda added a comment - +1 for David's proposal
              Hide
              dougiamas 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
              dougiamas 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              nebgor Aparup Banerjee added a comment -

              i like @category!

              Show
              nebgor Aparup Banerjee added a comment - i like @category!
              Hide
              dougiamas 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
              dougiamas 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
              dougiamas Martin Dougiamas added a comment -

              This now closed. Thanks all.

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

              Show
              dougiamas 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: