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

Port "Remove user role" action to D8 #214

Closed
wants to merge 1 commit into from
Closed

Port "Remove user role" action to D8 #214

wants to merge 1 commit into from

Conversation

stephenpurkiss
Copy link
Contributor

picking up from PR 182 - tidy & reroll so far

*
*/
class UserRoleRemove extends RulesActionBase
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opening braces should be on the same line, also elsewhere.

@stephenpurkiss
Copy link
Contributor Author

Hmm... seems my new version of phpstorm needs setting up again ;) Couple of points: fubhy noted on original PR "Please add a short description "Provides mocked user objects for the testRemoveExistingRole test." or somtehing like that." but I couldn't see where, let me know if this is still valid. Also katzilla noted test coverage may not be enough, let me know if more tests needed.

class UserRoleRemoveTest extends RulesEntityIntegrationTestBase {

use RulesUserIntegrationTestTrait;
/**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a newline before the comment.

@klausi
Copy link
Collaborator

klausi commented Sep 12, 2015

merged, thanks!

@klausi klausi closed this Sep 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants