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

mrrobot47 · 2018-08-28T17:11:27Z

Test: https://travis-ci.org/mrrobot47/easyengine/builds/421700140

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati · 2018-08-29T08:25:37Z

src/Site_Command.php

-		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 ) );


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.

rahulsprajapati · 2018-08-29T08:36:40Z

src/Site_Command.php

+		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,


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 );

rahulsprajapati · 2018-08-29T08:42:10Z

src/Site_Command.php

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


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 ).

rahulsprajapati · 2018-08-29T09:40:14Z

src/helper/hooks.php

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


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

rahulsprajapati · 2018-08-29T09:45:31Z

src/helper/hooks.php

+use EE\Dispatcher\CommandFactory;
+
+/**
+ * Add hook before the invocation of help command to appropriately handle the help for given site-type.


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.

rahulsprajapati · 2018-08-29T09:48:23Z

src/helper/hooks.php

+ */
+function Before_Help_Command( $args, $assoc_args ) {
+
+	if ( isset( $args[0] ) && 'site' === $args[0] ) {


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

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

rahulsprajapati · 2018-08-29T10:10:14Z

src/site-type/HTML.php

@@ -0,0 +1,357 @@
+<?php


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

rahulsprajapati · 2018-08-29T10:15:35Z

src/site-type/HTML.php

+	public function __construct() {
+
+		$this->level = 0;
+		pcntl_signal( SIGTERM, [ $this, "rollback" ] );


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 );

rahulsprajapati · 2018-08-29T10:16:32Z

src/site-type/HTML.php

+	 *
+	 * [--wildcard]
+	 * : Gets wildcard SSL .
+	 * [--type=<type>]


Add new blank line above this.

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati · 2018-08-29T10:55:36Z

site-command.php

@@ -13,4 +13,11 @@
 	require_once $autoload;
 }

+// Load utility functions.
+require_once 'src/helper/site-utils.php';


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

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati · 2018-08-30T07:57:38Z

src/helper/class-ee-site.php

+	/**
+	 * Shutdown function to catch and rollback from fatal errors.
+	 */
+	protected function shutDownFunction() {


why is this function name in camlecase?

Signed-off-by: Riddhesh Sanghvi <[email protected]>

mrrobot47 added 19 commits August 28, 2018 15:34

Move AcmePhp from core to site-command

9030635

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Move site related files from core to site-command

9dcdd84

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update composer dependencies

ab8c4d0

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update namespaces and change Site_Command to HTML site-type

74dbc3a

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Use constants where applicable

106134c

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Add site-type routing in site-command

7dbb02f

Signed-off-by: Riddhesh Sanghvi <[email protected]>

add help hook for site-type and register html site-type

2535117

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Move template from core to site-command

a2cbf6a

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update template path

b947d24

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update namespace and filenames

81e5159

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update namespace and filenames

1cf1d40

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Load site utility function file

b8ff0f0

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Add type namespace in shutdownhandler and letsencrypt class

2976ebe

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Remove unnecessary namespace

a7382df

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Add type namespace to Abstract class

da2cb12

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Add type namespace to letsencrypt class

ebd3eff

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update EE usage to \EE acc to new namespace

0178748

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update getting of args in help and set default type

a8af7f1

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update EE usage to \EE for letsencrypt class

5df0eaf

Signed-off-by: Riddhesh Sanghvi <[email protected]>

mrrobot47 requested review from kirtangajjar and rahulsprajapati August 28, 2018 17:11

mrrobot47 mentioned this pull request Aug 28, 2018

Move type routing to site-command EasyEngine/easyengine#1195

Merged

mrrobot47 added 2 commits August 28, 2018 23:34

Update execeptions according to namespace changes

000c413

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Merge branch 'develop' into refactor-for-type

a7f5d88

mrrobot47 removed the request for review from rahulsprajapati August 29, 2018 06:26

mrrobot47 changed the title ~~Refactor for --type support~~ WIP: Refactor for --type support Aug 29, 2018

mrrobot47 changed the title ~~WIP: Refactor for --type support~~ WIP: Refactor for --type support of ee site command Aug 29, 2018

rahulsprajapati self-requested a review August 29, 2018 07:21

mrrobot47 added 2 commits August 29, 2018 14:06

Move use statement to hooks file

eaefba4

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update doc comment

828e731

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati suggested changes Aug 29, 2018

View reviewed changes

mrrobot47 added 2 commits August 29, 2018 16:19

Update warning message

ed5c6e0

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update sprintf

eb2e1e4

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati reviewed Aug 29, 2018

View reviewed changes

mrrobot47 added 12 commits August 29, 2018 16:29

Remove TODO from code to track in seperate issue

69b6976

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update hook name and doc comment

4d9dc92

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Add early return in help hook

4f66fc2

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Rename html class file

75e55f7

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update help info in doc block

6dd18f8

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update path for require

18c462f

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Move shutdownfunction and common rollback to abstract class

077316c

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Move file loading to composer

b5c167d

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update loading sequence

e408a92

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Change hook addition

72444d8

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Remove test trigger of error

eb15ce5

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update hooks file to add hooks

a313af6

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati suggested changes Aug 30, 2018

View reviewed changes

mrrobot47 added 2 commits August 30, 2018 13:44

Update sprintf usage

482b4f8

Signed-off-by: Riddhesh Sanghvi <[email protected]>

Update function name

cd38bc7

Signed-off-by: Riddhesh Sanghvi <[email protected]>

rahulsprajapati previously approved these changes Aug 30, 2018

View reviewed changes

mrrobot47 removed the request for review from kirtangajjar August 30, 2018 08:22

mrrobot47 dismissed rahulsprajapati’s stale review via 1df34ce August 30, 2018 08:39

mrrobot47 force-pushed the refactor-for-type branch from 1df34ce to cd38bc7 Compare August 30, 2018 08:40

rahulsprajapati merged commit ebbc478 into EasyEngine:develop Aug 30, 2018

mrrobot47 deleted the refactor-for-type branch August 15, 2022 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mrrobot47 commented Aug 28, 2018 •

edited

Loading

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 29, 2018

rahulsprajapati Aug 30, 2018

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

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

Conversation

mrrobot47 commented Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrrobot47 commented Aug 28, 2018 •

edited

Loading