-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add options to create/remove groups via occ #10334
Conversation
Nice stuff! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We require you to sign off on your commits.
git commit --amend -s
Should do the trick
Also, could you run:
bash build/autoloaderchecker.sh
To make sure the autoloader is up to date?
core/Command/Group/Add.php
Outdated
@@ -0,0 +1,67 @@ | |||
<?php | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make the file strict?
declare(strict_types=1);
core/Command/Group/Add.php
Outdated
return 1; | ||
} else { | ||
$this->groupManager->createGroup($groupName); | ||
$group = $this->groupManager->get($groupName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createGroup
already returns the group no need for the get.
eb6bb4e
to
2cb8f22
Compare
Signed-off-by: Denis Mosolov <denismosolov@gmail.com>
78b48e4
to
0b18e2c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works 🚀
Too late to add this to NC14 IMO, so tagging this for 15. |
Thanks guys! |
#10334 (comment) -> @MorrisJobke @rullzer shall we revert this change? This is a new feature and we're past feature freeze. |
I'm fine with having this. Isolated new commands. |
@denismosolov In the future please wait for the approval for merge by others if it is stated like this in the comments. We are currently shortly before the RC 1 release and want to have this stable. |
For this one it's fine. |
Sorry for the accident. I didn't know what to do after getting 2 reviews. Just clicked the merge button because UI alowed me to do that. |
Add options to create and remove groups via command line.
Requested in #8317
Syntax:
occ group:add groupid
creates a new group, groupid - the new groups nameocc group:delete groupid
removes group, cannot remove the group named 'admin'@tterranigma , @EpeR1 what do you think?
Signed-off-by: Denis Mosolov denismosolov@gmail.com