Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Generated documentation - package tag or no package tag ? #590

Closed
3 tasks done
jrfnl opened this issue Nov 6, 2021 · 4 comments · Fixed by #606
Closed
3 tasks done

Generated documentation - package tag or no package tag ? #590

jrfnl opened this issue Nov 6, 2021 · 4 comments · Fixed by #606

Comments

@jrfnl
Copy link
Member

jrfnl commented Nov 6, 2021

Currently most classes have a @package tag and a number of them have a @subpackage tag.

These are not always applied, nor always applied consistently and the use of them impacts the generated documentation.

Old ApiGen generated docs, which lists the packages, classes, interfaces and exceptions in the menu:
image

Similarly, the phpDocumentor docs as will be generated for Requests 2.0.0 and later, which lists the namespaces and packages in the menu:
image

In the phpDocumentor, a stray Application package can be seen which is caused by a couple of files missing the @package tags.

Question:

Do we want to continue to use the @package tags ? or do we want to remove them in favour of the namespacing grouping related items ?

If we want to continue to use the tags...

... they should IMO be reviewed and made consistent.

In other words:

  • Missing @package tags need to be resolved.
  • All @package (and @subpackage) tags need to be reviewed.
  • Favour using hierarchical @package tags over the use of @subpackage as per PSR19.
  • Should the HookManager interface and the Hooks class be given an @package Requests\Events ?
  • In contrast to ApiGen, phpDocumentor does not list exceptions separately. Should all exceptions be given an @package Requests\Exceptions, with the HTTP ones getting @package Requests\Exceptions\Http ?
  • Are the classes which all have the @package Requests\Utilities tagged correctly ?
    image

If we are going to remove the tags....

Should the place of certain classes in the namespace hierarchy be changed to more closely match the previous package overview ?
I.e. move the Cookies class into the Cookies namespace and subdirectory, move the Proxy class into the Proxy namespace and subdirectory, move classes with the @subpackage Utilities into the Utilities namespace and subdirectory ?

@schlessera Do you have an opinion on this ?

@jrfnl jrfnl added this to the 2.0.0 milestone Nov 6, 2021
@jrfnl
Copy link
Member Author

jrfnl commented Nov 6, 2021

FYI: made a adjustment to the above text regarding the @subpackage tag which doesn't exist in the PSR19 proposal.

@schlessera
Copy link
Member

I've always found the naming of the @package annotation to be misleading. For my understanding, a "package" is the unit of distribution (it is being "packaged" to send around), so the way PHP Documentor uses this just seems technically wrong to me, as it seems to actually refer to a "module" or similar, not a "package".

That being said, as we already have the existing documentation use the @package annotation, I'd say we should stick with it and ensure it is consistent across the codebase.

I fully agree with your approach of doing so that you've described above in the checklist items.

@jrfnl
Copy link
Member Author

jrfnl commented Nov 8, 2021

@schlessera In that case these questions also need to be addressed:

  • Should the HookManager interface and the Hooks class be given an @package Requests\Events ?
  • In contrast to ApiGen, phpDocumentor does not list exceptions separately. Should all exceptions be given an @package Requests\Exceptions, with the HTTP ones getting @package Requests\Exceptions\Http ?
  • Are the classes which all have the @package Requests\Utilities tagged correctly ?

@schlessera
Copy link
Member

Should the HookManager interface and the Hooks class be given an @package Requests\Events ?

Yes, makes sense. An alternative would be something like @package Requests\Dispatching or @package Requests\EventDispatcher, but I don't feel strongly about any of them.

In contrast to ApiGen, phpDocumentor does not list exceptions separately. Should all exceptions be given an @package Requests\Exceptions, with the HTTP ones getting @package Requests\Exceptions\Http ?

I'd only go with @package Requests\Exceptions for all of them. I don't think we need to fully map the namespaces 1-to-1.

Are the classes which all have the @package Requests\Utilities tagged correctly ?

Yes, looks ok to me. These all seem to be "internal helpers" which are not meant to be directly used in consuming code.

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

Successfully merging a pull request may close this issue.

2 participants