Skip to content

Conversation

@deannahburke
Copy link

Change summary

Added JSON support to service-version clone command as reported in issue #1353.
NOTE: Unable to reproduce the ordering issue/race condition that the customer reported by running the list command immediately after the clone command. We ran this sequence multiple times, but each time the list was returned in the expected order, with the newly cloned service-version listed last.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

User Impact

  • Able to get JSON output from service-version clone command

Are there any considerations that need to be addressed for release?

No breaking changes.

As part of issue 1353, the service-version clone command is missing the option for JSON output and to use it in scripts the output need to be parsed.
This commit adds functionality to accept the --json argument to the service-version clone command and outputs the appropriate format.  Unit tests passing.
@kpfleming
Copy link
Contributor

The test failure on Windows is not related to the contents of this PR - that sort of 'race condition' failure happens sometimes on Windows but it hasn't been a priority for us to fix it. Feel free to just re-run the failing job and it will almost certainly succeed.

Copy link
Contributor

@kpfleming kpfleming left a comment

Choose a reason for hiding this comment

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

Some small changes needed, but overall this looks very good!

- feat(env): add detection for workspace ID ([#1560](https://github.com/fastly/cli/pull/1560))

### Bug fixes:
- fix(service-version): Add JSON support to service-version clone command to address issue ([#1353](https://github.com/fastly/cli/issues/1353))
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be listed as a feature, not a bug fix, even though lack of JSON support could be considered a bug if we had documentation claiming that all commands support JSON output :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the link in the CHANGELOG entry should be to the PR, not to the issue.

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