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

ALPHA7 #82

Closed
wants to merge 3 commits into from
Closed

ALPHA7 #82

wants to merge 3 commits into from

Conversation

xXSirGamesXx
Copy link

@xXSirGamesXx xXSirGamesXx commented Aug 24, 2017

Introduction

ALPHA7

Relevant issues

ALPHA7

Changes (ABC)

API changes

Behavioural changes

Configuration changes

Backwards compatibility

Follow-up

Tests

Copy link

@robske110 robske110 left a comment

Choose a reason for hiding this comment

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

You changed some stuff you shouldn't change,
also i don't think this is all that is required, did you test if the plugin works?

/**
* @return HereAuth
*/
public function getMain(){

Choose a reason for hiding this comment

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

wtf??

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove the type hint?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry i fixed this a little bit ago, but the branch i was using was the pw-hash branch so i tried pushing it onto this, ima restart

@@ -144,7 +144,7 @@ private function getHelpMessage(User $user){
$output = "Your HereAuth options:\n";
$mlp = $opts->maskLocPos;
if(!preg_match(/** @lang RegExp */
'#^((\?spawn\?)|((-)?[0-9]+,(-)?[0-9]+,(-)?[0-9]+))@([^/\\\\]+)$#', $mlp, $match)
'#^((\?spawn\?)|((\-)?[0-9]+,(\-)?[0-9]+,(\-)?[0-9]+))@([^/\\\\]+)$#', $mlp, $match)

Choose a reason for hiding this comment

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

why did you change the regex?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is actually the same, just more explicit, but unnecessary.

@@ -47,7 +47,7 @@ public function run(array $args, CommandSender $sender){
// $this->getMain()->getDataBase()->saveData($info, false);
// return "Account $args[0] is changed to $args[1]";
// }
$this->getMain()->getDataBase()->renameAccount($args[0], $args[1], function($result) use ($args, $sender){
$this->getMain()->getDataBase()->renameAccount($args[0], $args[1], function ($result) use ($args, $sender){

Choose a reason for hiding this comment

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

this is also unnecessary

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please get rid of IDE work.

@@ -36,11 +36,11 @@ protected function run(array $args, CommandSender $sender){
return $ev->getCancelMessage();
}
if($user !== null){
$user->resetAccount(function($success) use ($sender, $name){
$user->resetAccount(function ($success) use ($sender, $name){

Choose a reason for hiding this comment

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

did you let an IDE do this

$sender->sendMessage($success ? "Account $name has been unregistered." : "Account $name does not exist.");
});
}else{
$this->getMain()->getDataBase()->unregisterAccount($name, function($success) use ($sender, $name){
$this->getMain()->getDataBase()->unregisterAccount($name, function ($success) use ($sender, $name){

Choose a reason for hiding this comment

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

here too

/**
* @return HereAuth
*/
public function getMain(){

Choose a reason for hiding this comment

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

don't remove the typehints

Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is entirely unnecessary.

return $this->main;
}

public final function execute(CommandSender $sender, $commandLabel, array $args){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove the final modifier?

@@ -47,7 +47,7 @@ public function run(array $args, CommandSender $sender){
// $this->getMain()->getDataBase()->saveData($info, false);
// return "Account $args[0] is changed to $args[1]";
// }
$this->getMain()->getDataBase()->renameAccount($args[0], $args[1], function($result) use ($args, $sender){
$this->getMain()->getDataBase()->renameAccount($args[0], $args[1], function ($result) use ($args, $sender){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please get rid of IDE work.

/**
* @return HereAuth
*/
public function getMain(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove the type hint?

/**
* @return HereAuth
*/
public function getMain(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is entirely unnecessary.

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.

None yet

3 participants