Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

permissions : docBlock #5972

Merged
merged 2 commits into from

5 participants

@jmleroux

update docblock documentation

@jmleroux jmleroux commented on the diff
library/Zend/Permissions/Rbac/Rbac.php
@@ -110,6 +110,7 @@ public function getRole($objectOrName)
$it = new RecursiveIteratorIterator($this, RecursiveIteratorIterator::CHILD_FIRST);
foreach ($it as $leaf) {
+ /* @var RoleInterface $leaf */

For IDE autocomplete

@ralphschindler Collaborator

I think that is suppose to be /** @var RoleInterface $leaf */
when used inline, at least that i how PHPStorm does it.

I use PHPStorm and it actually use /**.
But it also understand /* and since Eclipse seems to prefer this last syntax, i thought it would be better.

@Maks3w Collaborator
Maks3w added a note

Really I prefer Docblock comment block syntax since @var is traditionally used in docblocks.

@Maks3w Collaborator
Maks3w added a note

Anyway this detail is not enough for do a change (even do a PR for this things)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Permissions/Rbac/Rbac.php
@@ -124,9 +125,10 @@ public function getRole($objectOrName)
/**
* Determines if access is granted by checking the role and child roles for permission.
*
- * @param RoleInterface|string $role
- * @param string $permission
+ * @param RoleInterface|string $role
+ * @param string $permission

Why did you removed the column alignment?

PHPStorm formatter
Is it a problem ?

@Ocramius Collaborator

Yeah, please revert this. The @throws addition is actually correct though

Is this the reason for CS error ? :cry:

I'd prefer it aligned tbh, but no problem, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@danizord danizord commented on the diff
library/Zend/Permissions/Rbac/Rbac.php
@@ -127,6 +128,7 @@ public function getRole($objectOrName)
* @param RoleInterface|string $role
* @param string $permission
* @param AssertionInterface|Callable|null $assert

You missed 1 more space to align with the @throws Exception\InvalidArgumentException as well :)

:alien: A little bit fussy on this one ! ^^

Well, your PR aims to fix docblock :smile:

@Maks3w Collaborator
Maks3w added a note

@danizord Alignment is not a valid reason for change a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Maks3w Maks3w was assigned by jmleroux
@Maks3w Maks3w merged commit 0cc76e0 into zendframework:master
@jmleroux jmleroux deleted the jmleroux:permissions-doc branch
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-permissions-rbac
@Maks3w Maks3w Merge pull request zendframework/zf2#5972 5e876f2
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-permissions-rbac
@Maks3w Maks3w Merge pull request zendframework/zf2#5972 in develop cf1b173
@weierophinney weierophinney referenced this pull request from a commit in zendframework/zend-permissions-rbac
@Maks3w Maks3w Merge pull request zendframework/zf2#5972 in master d60eac9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 14, 2014
  1. @jmleroux

    permissions : docBlock

    jmleroux authored
  2. @jmleroux
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 0 deletions.
  1. +2 −0  library/Zend/Permissions/Rbac/Rbac.php
View
2  library/Zend/Permissions/Rbac/Rbac.php
@@ -110,6 +110,7 @@ public function getRole($objectOrName)
$it = new RecursiveIteratorIterator($this, RecursiveIteratorIterator::CHILD_FIRST);
foreach ($it as $leaf) {
+ /* @var RoleInterface $leaf */

For IDE autocomplete

@ralphschindler Collaborator

I think that is suppose to be /** @var RoleInterface $leaf */
when used inline, at least that i how PHPStorm does it.

I use PHPStorm and it actually use /**.
But it also understand /* and since Eclipse seems to prefer this last syntax, i thought it would be better.

@Maks3w Collaborator
Maks3w added a note

Really I prefer Docblock comment block syntax since @var is traditionally used in docblocks.

@Maks3w Collaborator
Maks3w added a note

Anyway this detail is not enough for do a change (even do a PR for this things)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if ((is_string($objectOrName) && $leaf->getName() == $objectOrName) || $leaf == $objectOrName) {
return $leaf;
}
@@ -127,6 +128,7 @@ public function getRole($objectOrName)
* @param RoleInterface|string $role
* @param string $permission
* @param AssertionInterface|Callable|null $assert

You missed 1 more space to align with the @throws Exception\InvalidArgumentException as well :)

:alien: A little bit fussy on this one ! ^^

Well, your PR aims to fix docblock :smile:

@Maks3w Collaborator
Maks3w added a note

@danizord Alignment is not a valid reason for change a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ * @throws Exception\InvalidArgumentException
* @return bool
*/
public function isGranted($role, $permission, $assert = null)
Something went wrong with that request. Please try again.