Magento Quality Patches - improvement of dev experience #61
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |
| "name": "magento/magento-cloud-patches", | ||
| "description": "Provides critical fixes for Magento 2 Enterprise Edition", | ||
| "type": "magento2-component", | ||
| "version": "1.0.6", | ||
| "license": "OSL-3.0", | ||
| "require": { | ||
| "php": "^7.0", | ||
|
|
@@ -14,6 +14,7 @@ | |
| "symfony/dependency-injection": "^3.3||^4.3", | ||
| "symfony/process": "^2.1||^4.1", | ||
| "symfony/proxy-manager-bridge": "^3.3||^4.3", | ||
| "symfony/yaml": "^3.3||^4.0", | ||
| "monolog/monolog": "^1.16", | ||
| "magento/quality-patches": "^1.0.0" | ||
|
Member
There was a problem hiding this comment. This causes circular dependency and must be moved to another package
Contributor
Author
There was a problem hiding this comment. The packages have a dependency on each other and the composer resolves them without issues. Why it should be moved?
Member
There was a problem hiding this comment. Having circular dependency is never good idea, in some cases, it lead to neverending dependency resolving and other problems. This may be either included in ECE-Tools or MCC
Member
There was a problem hiding this comment. And circular dependency consieded as "bad smell" from Architecture Design standpoint
Contributor
Author
There was a problem hiding this comment.
I agree with you, in case of recursive read it can lead to neverending resolving.
Contributor
Author
There was a problem hiding this comment. "magento/quality-patches" dependency was moved to suggest section |
||
| }, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace Magento\CloudPatches; | ||
|
|
||
| use Composer\Composer; | ||
| use Magento\CloudPatches\Command; | ||
| use Psr\Container\ContainerInterface; | ||
|
|
||
| /** | ||
| * @inheritdoc | ||
| */ | ||
| class ApplicationEce extends \Symfony\Component\Console\Application | ||
| { | ||
| /** | ||
| * @var ContainerInterface | ||
| */ | ||
| private $container; | ||
|
|
||
| /** | ||
| * @param ContainerInterface $container | ||
| */ | ||
| public function __construct(ContainerInterface $container) | ||
| { | ||
| $this->container = $container; | ||
|
|
||
| parent::__construct( | ||
| $container->get(Composer::class)->getPackage()->getPrettyName(), | ||
| $container->get(Composer::class)->getPackage()->getPrettyVersion() | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * @inheritdoc | ||
| */ | ||
| protected function getDefaultCommands() | ||
| { | ||
| return array_merge(parent::getDefaultCommands(), [ | ||
| $this->container->get(Command\Ece\Apply::class), | ||
| $this->container->get(Command\Ece\Revert::class), | ||
| $this->container->get(Command\Status::class) | ||
| ]); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,19 +8,15 @@ | |
| namespace Magento\CloudPatches\Command; | ||
|
|
||
| use Magento\CloudPatches\App\RuntimeException; | ||
| use Magento\CloudPatches\Command\Process\ApplyOptional; | ||
| use Magento\CloudPatches\Composer\MagentoVersion; | ||
| use Psr\Log\LoggerInterface; | ||
| use Symfony\Component\Console\Input\InputArgument; | ||
| use Symfony\Component\Console\Input\InputInterface; | ||
| use Symfony\Component\Console\Output\OutputInterface; | ||
|
|
||
| /** | ||
| * Patch apply command (OnPrem). | ||
| */ | ||
| class Apply extends AbstractCommand | ||
| { | ||
|
|
@@ -30,35 +26,15 @@ class Apply extends AbstractCommand | |
| const NAME = 'apply'; | ||
|
|
||
| /** | ||
| * List of patches to apply. | ||
| */ | ||
| const ARG_LIST_OF_PATCHES = 'list-of-patches'; | ||
|
|
||
| /** | ||
| * @var ApplyOptional | ||
| */ | ||
| private $applyOptional; | ||
|
|
||
| /** | ||
| * @var LoggerInterface | ||
| */ | ||
|
|
@@ -70,25 +46,16 @@ class Apply extends AbstractCommand | |
| private $magentoVersion; | ||
|
|
||
| /** | ||
| * @param ApplyOptional $applyOptional | ||
| * @param LoggerInterface $logger | ||
| * @param MagentoVersion $magentoVersion | ||
| */ | ||
| public function __construct( | ||
| ApplyOptional $applyOptional, | ||
| LoggerInterface $logger, | ||
| MagentoVersion $magentoVersion | ||
| ) { | ||
| $this->applyOptional = $applyOptional; | ||
| $this->logger = $logger; | ||
| $this->magentoVersion = $magentoVersion; | ||
|
|
||
|
|
@@ -101,17 +68,11 @@ public function __construct( | |
| protected function configure() | ||
| { | ||
| $this->setName(self::NAME) | ||
| ->setDescription('Applies patches. The list of patches should pass as a command argument') | ||
| ->addArgument( | ||
| self::ARG_LIST_OF_PATCHES, | ||
|
Member
There was a problem hiding this comment. This is BC change, it should not go into patch version release
Contributor
Author
There was a problem hiding this comment. It's a new command - ./magento-patches apply,
Contributor
Author
There was a problem hiding this comment. @shiftedreality or you mean removing of --git-installation option? |
||
| InputArgument::IS_ARRAY | InputArgument::REQUIRED, | ||
| 'List of patches to apply' | ||
| ); | ||
|
|
||
| parent::configure(); | ||
|
|
@@ -122,23 +83,10 @@ protected function configure() | |
| */ | ||
| public function execute(InputInterface $input, OutputInterface $output) | ||
| { | ||
| $this->logger->notice($this->magentoVersion->get()); | ||
|
|
||
| try { | ||
| $this->applyOptional->run($input, $output); | ||
| } catch (RuntimeException $e) { | ||
| $output->writeln('<error>' . $e->getMessage() . '</error>'); | ||
| $this->logger->error($e->getMessage()); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| <?php | ||
| /** | ||
| * Copyright © Magento, Inc. All rights reserved. | ||
| * See COPYING.txt for license details. | ||
| */ | ||
| declare(strict_types=1); | ||
|
|
||
| namespace Magento\CloudPatches\Command\Ece; | ||
|
|
||
| use Magento\CloudPatches\App\RuntimeException; | ||
| use Magento\CloudPatches\Command\AbstractCommand; | ||
| use Magento\CloudPatches\Command\Process\ApplyLocal; | ||
| use Magento\CloudPatches\Command\Process\Ece\ApplyOptional; | ||
| use Magento\CloudPatches\Command\Process\ApplyRequired; | ||
| use Magento\CloudPatches\Composer\MagentoVersion; | ||
| use Psr\Log\LoggerInterface; | ||
| use Symfony\Component\Console\Input\InputInterface; | ||
| use Symfony\Component\Console\Output\OutputInterface; | ||
|
|
||
| /** | ||
| * Patch apply command (Cloud). | ||
| */ | ||
| class Apply extends AbstractCommand | ||
| { | ||
| /** | ||
| * Command name. | ||
| */ | ||
| const NAME = 'apply'; | ||
|
|
||
| /** | ||
| * @var ApplyOptional | ||
| */ | ||
| private $applyOptional; | ||
|
|
||
| /** | ||
| * @var ApplyRequired | ||
| */ | ||
| private $applyRequired; | ||
|
|
||
| /** | ||
| * @var ApplyLocal | ||
| */ | ||
| private $applyLocal; | ||
|
|
||
| /** | ||
| * @var LoggerInterface | ||
| */ | ||
| private $logger; | ||
|
|
||
| /** | ||
| * @var MagentoVersion | ||
| */ | ||
| private $magentoVersion; | ||
|
|
||
| /** | ||
| * @param ApplyRequired $applyRequired | ||
| * @param ApplyOptional $applyOptional | ||
| * @param ApplyLocal $applyLocal | ||
| * @param LoggerInterface $logger | ||
| * @param MagentoVersion $magentoVersion | ||
| */ | ||
| public function __construct( | ||
| ApplyRequired $applyRequired, | ||
| ApplyOptional $applyOptional, | ||
| ApplyLocal $applyLocal, | ||
| LoggerInterface $logger, | ||
| MagentoVersion $magentoVersion | ||
| ) { | ||
| $this->applyRequired = $applyRequired; | ||
| $this->applyOptional = $applyOptional; | ||
| $this->applyLocal = $applyLocal; | ||
| $this->logger = $logger; | ||
| $this->magentoVersion = $magentoVersion; | ||
|
|
||
| parent::__construct(self::NAME); | ||
| } | ||
|
|
||
| /** | ||
| * @inheritDoc | ||
| */ | ||
| protected function configure() | ||
| { | ||
| $this->setName(self::NAME) | ||
| ->setDescription('Applies patches (Magento Cloud only)'); | ||
|
|
||
| parent::configure(); | ||
| } | ||
|
|
||
| /** | ||
| * @inheritDoc | ||
| */ | ||
| public function execute(InputInterface $input, OutputInterface $output) | ||
| { | ||
| $this->logger->notice($this->magentoVersion->get()); | ||
|
|
||
| try { | ||
| $this->applyRequired->run($input, $output); | ||
| $this->applyOptional->run($input, $output); | ||
| $this->applyLocal->run($input, $output); | ||
| } catch (RuntimeException $e) { | ||
| $output->writeln('<error>' . $e->getMessage() . '</error>'); | ||
| $this->logger->error($e->getMessage()); | ||
|
|
||
| return self::RETURN_FAILURE; | ||
| } catch (\Exception $e) { | ||
| $this->logger->critical($e); | ||
|
|
||
| throw $e; | ||
| } | ||
|
|
||
| return self::RETURN_SUCCESS; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is redundant since ApplicationEce already Cloud-aware
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.
Yes, but there is CloudCollector service that is used by both applications and we need to define what is the environment to set patch type optional or required https://github.com/magento-mpi/magento-cloud-patches/blob/65837e0a187dfa282e47be67c7dc1cd349643709/src/Patch/Collector/CloudCollector.php#L94