Posted by -enzo- on September 25, 2012 at 4:56am
6 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | documentation |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | Configuration system |
Issue Summary
Update documentation definition for functions
Comments
#1
#2
Thanks for this patch, I miss the @return values in config greatly.
Please remove the @return on castValue, it's an array or a string not a string but hopefully #1653026: All configuration values are stored as strings will fix that for once and all.
Past that the only problem I see here is the wording on get() -- I will think on it.
#3
Leave castValue alone for now
#4
#5
#6
This can't be right:
/*** Returns the name of this configuration object.
+ *
+ * @return Drupal\Core\Config\Config
+ * The configuration object.
*/
public function getName() {
Either the first line or the added @return is incorrect. I didn't look through the rest of the patch, but it looks like it was just generated by pasting the same @return for each function -- it needs to be done a bit more carefully. Thanks!
#7
Fixed documentation on getName().
#8
This looks good to me now and thanks for the work enzo and socketwench!
#9
Jennifer's got this one. :)
#10
I've committed this patch, as everything in it looks good.
But the file is not still up to our coding standards... As a follow-up, would someone like to go through and fix these items:
a) Berb tense in the function first lines? For instance:
/*** Dispatch a config event.
*/
protected function notify($config_event_name) {
Should be Dispatches. There are several others that should be fixed.
b) In the get() method, the explanation should go before the @param, not between the @param and the @return, and the @see should go at the end.
c) There are also a couple of minor wording/grammar things that could be fixed, such as:
/*** Sets value in this config object.
Should be "Sets a value...".
d) The castValue() method needs a @return but doesn't have one (it has two @params, and I think the second should be @return).
e) After this patch, not all the param/returns have data types. still.
#11
Oh, not my issue now. :)
#12
Do not try to fix d). castValue is scheduled to be removed, it's an abomination, so please disregard that one.
#13
I'll try to update the patch after work today.