Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Fix theme push --development --json to output the proper exit code #1769

Merged
merged 2 commits into from
Nov 18, 2021

Conversation

karreiro
Copy link
Contributor

@karreiro karreiro commented Nov 17, 2021

Fix theme push --development --json command to output the proper exit code when an error occurs (#1353).

WHY are these changes introduced?

These changes aim to output exit code 1 when anything wrong happens during a theme push operation.

WHAT is this pull request doing?

This PR extracts error reporting logic from ShopifyCLI::Theme::Syncer and delegates it to ShopifyCLI::Theme::Syncer::ErrorReporter. At the error reporter level, we have the mechanism to identify if any error report has happened at that session.

Then, at the Theme::Command::Push level, we check if the syncer has reported any errors, and we finally provide the proper exit code.

How to test your changes?

  1. Run the theme push command with the following parameters:
shopify theme push --development --json
  1. Check exit code:
echo "Exit code: $?"

Here's the previous behavior:

Here's how it actual behavior (with this PR applied):

Post-release steps

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

@karreiro karreiro requested review from macournoyer, a team, hannachen and gonzaloriestra and removed request for a team November 17, 2021 16:44
Copy link
Contributor

@macournoyer macournoyer left a comment

Choose a reason for hiding this comment

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

Wowow! 👍 Nice refactoring too, again 🧹

@karreiro karreiro merged commit 8ff02de into main Nov 18, 2021
@karreiro karreiro deleted the fix-1353 branch November 18, 2021 08:47
@karreiro karreiro linked an issue Nov 19, 2021 that may be closed by this pull request
@jesalerno84 jesalerno84 mentioned this pull request Nov 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shopify push --json should not output non-json output to STDOUT
2 participants