Skip to content
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

Remove permission interface #16

Merged
merged 4 commits into from
Jun 25, 2014
Merged

Remove permission interface #16

merged 4 commits into from
Jun 25, 2014

Conversation

bakura10
Copy link
Member

ping @danizord @jmleroux @arekkas

Any other idea about what we could do?

@aeneasr
Copy link
Contributor

aeneasr commented Jun 22, 2014

There are two typecasts in the default role implementation:

should they be kept, now that the permissioninterface is gone?

@bakura10
Copy link
Member Author

I'd prefer to keep them. They make sure that numbers are cast to string to (though naming permissions with number would be a bit strange :d)

@bakura10
Copy link
Member Author

And it would allow someone using ZfcRbac to still be able to use those built-in classes.

@aeneasr
Copy link
Contributor

aeneasr commented Jun 22, 2014

Maybe do something like

    /**
     * {@inheritDoc}
     */
    public function hasPermission($permission)
    {
        if (!is_string($perimssion)) {
            throw new InvalidArgumentException;
        }
        return isset($this->permissions[$permission]);
    }

?

@aeneasr
Copy link
Contributor

aeneasr commented Jun 22, 2014

But sure, I'm fine with keeping them. Just wanted to ask to see if you missed this

bakura10 added a commit that referenced this pull request Jun 25, 2014
Remove permission interface
@bakura10 bakura10 merged commit 514aee7 into master Jun 25, 2014
@bakura10
Copy link
Member Author

I'm merging. I don't know what to include now into this library.

@arekkas , the CARBAC feature would actually something that would be here or in ZfcRbac?

@aeneasr
Copy link
Contributor

aeneasr commented Jun 25, 2014

Yes absolutely :)

@bakura10
Copy link
Member Author

Absolutely what?

@aeneasr
Copy link
Contributor

aeneasr commented Jun 25, 2014

@arekkas , the CARBAC feature would actually something that would be here or in ZfcRbac?

CaRBAC is something for directly here or ZfcRbac

@bakura10 bakura10 deleted the 2.0 branch November 5, 2015 08:27
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