-
Notifications
You must be signed in to change notification settings - Fork 67
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
DOC Document PHPDoc as a PHP coding standard #448
DOC Document PHPDoc as a PHP coding standard #448
Conversation
863a3d4
to
f30dfa8
Compare
|
||
PHPDocs are not only useful when looking at the source code, but are also used in the API documentation at <https://api.silverstripe.org>. | ||
|
||
- All [public API](/project_governance/public_api) should have a PHPDoc to describe its purpose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- All [public API](/project_governance/public_api) should have a PHPDoc to describe its purpose. |
We haven't agreed on this.
I don't really like this because if you have clear method names then the docblock description is a redundant duplication
I do notice the word "should" instead of "must", though then it's just kind of vague and it's not going to resolve any arguments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word "must" doesn't appear anywhere in here - I think you're worrying a little too much about that word (or else this whole document is effectively pointless :p)
I've added extra criteria - if the api name and signature explain everything you need to know, then I guess we can leave a description off. But really, most of the time it should be added. I don't think this is controversial.
PHPDocs are not only useful when looking at the source code, but are also used in the API documentation at <https://api.silverstripe.org>. | ||
|
||
- All [public API](/project_governance/public_api) should have a PHPDoc to describe its purpose. | ||
- All `DataObject` and `Extension` subclasses should have `@method` annotations in their PHPDoc for relations (`has_one`, `has_many`, etc). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- All `DataObject` and `Extension` subclasses should have `@method` annotations in their PHPDoc for relations (`has_one`, `has_many`, etc). | |
- All `DataObject` and `Extension` subclasses must have `@method` annotations in their PHPDoc for relations (`has_one`, `has_many`, etc). |
Since it's being enforced by CI it's a must
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word "must" doesn't appear anywhere else in this document. Really it shouldn't matter whether these are enforce by CI or not. But I'll change this word since in this case yes, I suppose it would fail peer review (CI not-withstanding) if the annotation is missing.
f30dfa8
to
93a7a77
Compare
Description
Documents the need for PHPDoc in general (which I was surprised was missing), and the need for
@method
annotations in particular.Issues
Pull request checklist