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

Refactor for --type support of ee site command #104

Merged
merged 41 commits into from
Aug 30, 2018

Conversation

mrrobot47
Copy link
Member

@mrrobot47 mrrobot47 commented Aug 28, 2018

Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
@mrrobot47 mrrobot47 removed the request for review from rahulsprajapati August 29, 2018 06:26
@mrrobot47 mrrobot47 changed the title Refactor for --type support WIP: Refactor for --type support Aug 29, 2018
@mrrobot47 mrrobot47 changed the title WIP: Refactor for --type support WIP: Refactor for --type support of ee site command Aug 29, 2018
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
if ( $this->ssl ) {
$info[] = [ 'SSL Wildcard', $this->ssl_wildcard ? 'Yes': 'No' ];
if ( isset( self::$instance->site_types[ $name ] ) ) {
EE::warning( sprintf( '%s site-type has already been previously registered by %s. It will be over-written by the new package class %s. Please update your packages to resolve this.', $name, self::$instance->site_types[ $name ], $callback ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be over-written by th

It's overridden by the other package class ....

Update message as above since it's already overridden after this message and we are just adding a warning here as information not asking yet.

if ( ! isset( $site_types[ $type ] ) ) {
$error = sprintf(
"'%s' is not a registered site type of 'ee site --type=%s'. See 'ee help site --type=%s' for available subcommands.",
$type,
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use %1$s to all %s places and just add one $type in args.

You'll have to use ' single quote for this since the last $s will be used as variable if used double quotes.

sprintf(
	'\'%1$s\' is not a registered site type of \'ee site --type=%1$s\'. See \'ee help site --type=%1$s\' for available subcommands.',
	$type
);

private function rollback() {
// TODO: get type from config file as below
// $config_type = EE::get_config('type');
// $type = empty( $config_type ) ? 'html' : $config_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this type of todo in some task/issue. Do not add commented code in repo. You can add all the details in issue. ( with code and line no mentioned ).

/**
* Add hook before the invocation of help command to appropriately handle the help for given site-type.
*/
function Before_Help_Command( $args, $assoc_args ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add functionality related function name. i.e ee_site_help_cmd_routing.

use EE\Dispatcher\CommandFactory;

/**
* Add hook before the invocation of help command to appropriately handle the help for given site-type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this comment.

/* Callback function of `before_invoke:help` hook: Add routing for "ee help site" command before the invocation of help command.

And the argument comment is missing.

*/
function Before_Help_Command( $args, $assoc_args ) {

if ( isset( $args[0] ) && 'site' === $args[0] ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do early return if nothing else is going to execute after this condition.

if ( ( ! isset( $args[0] ) ) || ( 'site' !== $args[0] ) ) {
	return;
}

@@ -0,0 +1,357 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

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

The file name should be in small char. html.php

public function __construct() {

$this->level = 0;
pcntl_signal( SIGTERM, [ $this, "rollback" ] );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this constructor has some common code for all site type. Can this be go in parent/abstract class constructor or method and call it here by parent::__construct(); or $this->function_name_which_has_this_code().

And make rollback as abstract so that every class has some rollback operation. abstract public function rollback( $args, $assoc_args );

*
* [--wildcard]
* : Gets wildcard SSL .
* [--type=<type>]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add new blank line above this.

Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
site-command.php Outdated
@@ -13,4 +13,11 @@
require_once $autoload;
}

// Load utility functions.
require_once 'src/helper/site-utils.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

add file using abs path.
Use DIR . '/src/helper/site-utills.php';
Do same for other places also.

Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
/**
* Shutdown function to catch and rollback from fatal errors.
*/
protected function shutDownFunction() {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this function name in camlecase?

Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
Signed-off-by: Riddhesh Sanghvi <riddheshsanghvi96@gmail.com>
@mrrobot47 mrrobot47 removed the request for review from kirtangajjar August 30, 2018 08:22
@rahulsprajapati rahulsprajapati merged commit ebbc478 into EasyEngine:develop Aug 30, 2018
@mrrobot47 mrrobot47 deleted the refactor-for-type branch August 15, 2022 16:13
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