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

          Issue Links

            Activity

            Hide
            mudrd8mz David Mudrak added a comment -

            ping pong

            Please comment and vote as will

            Show
            mudrd8mz David Mudrak 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 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
            mudrd8mz 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
            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 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            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 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
            mudrd8mz 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
            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 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
            mudrd8mz 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
            mudrd8mz 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
            mudrd8mz 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
            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 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
            mudrd8mz 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
            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: