Skip to content

Conversation

@pboivin
Copy link
Contributor

@pboivin pboivin commented Jun 4, 2021

Description

This is a draft PR to get some feedback on PHP code docblocks. Specifically, I want to discuss the potential value to be added to the API documentation. I understand that code comments can be a bit controversial as a topic and I'm genuinely interested to hear all personal opinions :)

The change presented here is to move the descriptions for HasMedias public methods, from the main docs to the code. This has the effect of enriching the API docs with descriptions and type information. For comparison :

Trait overview — current vs. updated

overview

Method details — current vs. updated

method-details

Also, the annotated methods are removed from the main docs and replaced with a link to the API docs. From my perspective, this makes the methods a bit less visible in the context of the whole documentation. On the other hand, it makes the actual documentation more inline with the code and perhaps easier to maintain in the long term.

I'll admit that backfilling the docblocks is boring... I think that a good place to start would be the model and repository behaviors (traits). The annotations would trickle down to all of the consuming entities in the API documentation.

Curious to know what you think!

Related

#929

@pboivin pboivin changed the title Add HasMedias docblocks Add HasMedias docblocks (WIP) Jun 4, 2021
@ifox
Copy link
Member

ifox commented Jun 4, 2021

This is great @pboi20! My main opinion regarding code comments is that I prioritize explicitness and eloquence of the code itself first, and that code comments are only relevant if they provide additional value, wether it is to provide more context or be helpful for static analysis.

The changes you propose here around the HasMedias trait make total sense to me. I think we can improve this overall gradually with time, with a focus on the methods that are used in userland (as in public methods, though the surface of public methods is currently quite large compared to the top level APIs most users are dealing with, to allow for customization).

@pboivin
Copy link
Contributor Author

pboivin commented Jun 4, 2021

@ifox Thanks for the feedback! Totally agree on code explicitness, eloquence and the focus on providing context (ie. Why instead of How). I would add that, in my mental model, docblocks are special and more akin to documentation than any other form of comment.

I'll keep this PR open as a draft and try to extend it to all model and repository behaviors.

PS. If anyone has any interest in this sort of housekeeping, let me know! We'll find a way to coordinate :)

@pboivin pboivin changed the title Add HasMedias docblocks (WIP) Add model behaviors docblocks (WIP) Jun 5, 2021
@pboivin pboivin changed the title Add model behaviors docblocks (WIP) Update model and repository behaviors docblocks Jun 7, 2021
@pboivin pboivin marked this pull request as ready for review June 7, 2021 20:07
@pboivin
Copy link
Contributor Author

pboivin commented Jun 7, 2021

@ifox This covers all public methods for model and repository behaviors. It focuses on the model behaviors — the repository behavior docblocks were already in good shape.

In many cases, only type annotations were added. This helps with IDE "intellisense" and adds click-through ability in the API docs for internal entities.

The trait annotations extend nicely to all models and repositories in the API docs. With time, we can shape the Twill API docs to become as descriptive and interactive as the Laravel counterparts :)

@ifox ifox merged commit 17390bb into area17:2.x Jun 28, 2021
@pboivin pboivin deleted the add-hasmedias-docblocks branch June 28, 2021 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants