[HttpFoundation][DX] MockArraySessionStorage: phpdocs update #22139
Conversation
MacDada
commented
Mar 24, 2017
MacDada
commented
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| License | MIT |
MacDada
changed the title
MacDada
changed the title
| @@ -58,13 +58,11 @@ class MockArraySessionStorage implements SessionStorageInterface | |||
| protected $metadataBag; | |||
|
|
|||
| /** | |||
| * @var array | |||
| * @var array|SessionBagInterface | |||
There was a problem hiding this comment.
Seems wrong. It's more like SessionBagInterface[].
There was a problem hiding this comment.
I guess there is no perfect solution. It is an array, so the first type declaration is true. And it can be treated as list of SessionBagInterface, so the other one is also true.
In such cases, I prefer to have both types defined, as both are useful. But I could delete array if you consider it unnecessary.
There was a problem hiding this comment.
Oh, right, nvm, I'm blind ;)
| @@ -75,8 +73,6 @@ public function __construct($name = 'MOCKSESSID', MetadataBag $metaBag = null) | |||
| } | |||
|
|
|||
| /** | |||
| * Sets the session data. | |||
There was a problem hiding this comment.
As the commit message says:
remove phpdoc title that says nothing more than method name
The title duplicates what's being said in the name of the function. It doesn't add any value to the developer.
There was a problem hiding this comment.
But then the @param below can be removed as well... and thousands of other phpdocs. Let's not do that. A good time will be when we can really get rid of the vast majority of current phpdocs (but on all the code base at once). Right now, that would a lot of work that does not beings value and will give the core team a lot of work when merging branches. So, I'm 👎 for these changes.
There was a problem hiding this comment.
But then the @param below can be removed as well...
Yep, I would remove it.
and thousands of other phpdocs.
Probably, yeah.
A good time will be when we can really get rid of the vast majority of current phpdocs (but on all the code base at once).
I guess, with php7 and scalar typehinting?
I'm 👎 for these changes.
You mean this one change or other changes as well? I can rewrite the PR of course.
There was a problem hiding this comment.
I would only keep the @var array|SessionBagInterface[] change, which adds value.
…pletion would work
MacDada
force-pushed
the
MockArraySessionStorage_phpdocs_update
branch
from
f5631de
to
967f7a7
Compare
|
Thank you @MacDada. |
…date (MacDada) This PR was merged into the 2.7 branch. Discussion ---------- [HttpFoundation][DX] MockArraySessionStorage: phpdocs update | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT Commits ------- 967f7a7 MockArraySessionStorage: updated phpdoc for $bags so that IDE autocompletion would work
