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

Move functionality from Artifact class to ArtifactCommand class #76

Closed
AlexSkrypnyk opened this issue Mar 11, 2024 · 0 comments · Fixed by #80
Closed

Move functionality from Artifact class to ArtifactCommand class #76

AlexSkrypnyk opened this issue Mar 11, 2024 · 0 comments · Fixed by #80
Assignees

Comments

@AlexSkrypnyk
Copy link
Member

AlexSkrypnyk commented Mar 11, 2024

#42 allows us to further improve the architecture of this package.

BLOCKED by #64


We now need to reconcile Artifact and ArtifactCommand classes.

This ticket is about moving the functionality of the Artifact to ArtifactCommand.

Please note that there is be a follow-up ticket to replace the logger with monolog.

Solution direction.

  1. Merge the functionality of Artifact to ArtifactCommand. The Artifact::artifact() should become ArtifactCommand::processArtifact() and be called from ArtifactCommand::execute().

  2. Update `ArtifactCommand::execute() to correctly return exit codes on exception.
    The code should look like:

  protected function execute(InputInterface $input, OutputInterface $output): int {
    $this->logger = self::logInit($output);

    $this->checkRequirements();

    try {
      $this->processArtifact();
    }
    catch (\Exception $e) {
      $output->writeln('<error>' . $e->getMessage() . '</error>');

      return Command::FAILURE;
    }

    return Command::SUCCESS;
  }
@AlexSkrypnyk AlexSkrypnyk changed the title Simplify ArtifactCommand and move dependencies into Artifact class Move functionality from Artifact class to ArtifactCommand class Mar 11, 2024
AlexSkrypnyk pushed a commit that referenced this issue Mar 19, 2024
…s. (#80)

* Move functionality from Artifact class to ArtifactCommand class.

* Update coverage test name.

* Check branch exisiting before do remove.

* Revert to standard app entry
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 a pull request may close this issue.

2 participants