Skip to content
This repository was archived by the owner on May 31, 2024. It is now read-only.

feat: Add JSON as a CLI output format (#215)#309

Merged
dtzeng merged 2 commits into
mainfrom
jsonformat
Feb 22, 2022
Merged

feat: Add JSON as a CLI output format (#215)#309
dtzeng merged 2 commits into
mainfrom
jsonformat

Conversation

@dtzeng
Copy link
Copy Markdown
Contributor

@dtzeng dtzeng commented Feb 4, 2022

Issue #, if available: #215

Description of Changes

  • Fix the agc configure format command as that was never working as-is.
  • Added a new json format class that extends the formatting interface
  • Added unit tests/reshuffled some shared types

Description of how you validated changes

Added unit tests

Testing agc configure format manually

dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc configure format text
2022-02-04T14:21:24-08:00 𝒊  Setting default format to: 'text'
dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc workflow describe hello
2022-02-04T14:21:35-08:00 𝒊  Describing workflow 'hello'.
WORKFLOW	hello	workflows/hello/hello.wdl	wdl	1.0
dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc configure format table
2022-02-04T14:21:38-08:00 𝒊  Setting default format to: 'table'
dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc workflow describe hello
2022-02-04T14:21:39-08:00 𝒊  Describing workflow 'hello'.
Name	TypeLanguage	TypeVersion	Source
hello	wdl		1.0		workflows/hello/hello.wdldtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ 
dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc configure format json
2022-02-04T14:21:48-08:00 𝒊  Setting default format to: 'json'
dtzeng@u787d729b615b5f:/workplace/dtzeng/amazon-genomics-cli/examples/demo-wdl-project$ agc workflow describe hello
2022-02-04T14:21:49-08:00 𝒊  Describing workflow 'hello'.
{
	"Name": "hello",
	"TypeLanguage": "wdl",
	"TypeVersion": "1.0",
	"Source": "workflows/hello/hello.wdl"
}

tested json output of other commands can be seen in this doc: https://quip-amazon.com/klA5AaAOPF62/agc-json-format

Checklist

  • If this change would make any existing documentation invalid, I have included those updates within this PR
  • I have added unit tests that prove my fix is effective or that my feature works
  • I have linted my code before raising the PR
  • Title of this Pull Request follows Conventional Commits standard: https://www.conventionalcommits.org/en/v1.0.0/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@dtzeng dtzeng temporarily deployed to slack February 4, 2022 22:59 Inactive
@dtzeng dtzeng changed the title Add JSON as a CLI output format (#215) feat: Add JSON as a CLI output format (#215) Feb 4, 2022
return fmt.Errorf("a single format value must be provided")
}
format := format.FormatterType(o.formatContextVars.format)
format := format.FormatterType(args[0])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please pull the args[0] value into a meaningful variable, especially since we reuse it across the code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We aren't actually using args[0] within the same function
First we validate the args within Validate(args) then we pull out args[0] and assign it to opts.format

Comment thread packages/cli/internal/pkg/cli/configure_format_test.go Outdated
@dtzeng dtzeng requested a review from tneely February 7, 2022 22:12
@dtzeng dtzeng merged commit 2d6ad0c into main Feb 22, 2022
@dtzeng dtzeng deleted the jsonformat branch February 22, 2022 22:06
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.

3 participants