Skip to content

Conversation

@matt-condon
Copy link
Collaborator

Proposed changes

Jira ticket: CLOUDP-184341

In releases 1.7.x, with the integration of the new automated Go SDK, we introduced breaking change to several "list" commands that modified the response structure for "--output json"

This PR is to introduces a flag "--compact" to revert the behaviour for the same specific set of list commands, so that users can modify their scripts with the "-c" flag to align their scripts with the previous behavior

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated test/README.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

@matt-condon matt-condon requested a review from a team June 15, 2023 10:15
@matt-condon matt-condon force-pushed the fix/compactListResults branch from 62e00de to 60de93b Compare June 15, 2023 10:20
@matt-condon matt-condon requested a review from a team as a code owner June 15, 2023 10:20
@matt-condon matt-condon force-pushed the fix/compactListResults branch from f9b657e to 786d382 Compare June 15, 2023 11:24
- Type
- Required
- Description
* - -c, --compact
Copy link
Member

Choose a reason for hiding this comment

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

I might be missing context. Did we agree for this flag to be present in docs (not hidden?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Jakub flagged that he wanted a note in --help for affected commands, which for me implies that it should be present in docs

Add note to --help for affected commands that provide a short explanation for the relevant (affected) CLI versions

- Type
- Required
- Description
* - -c, --compact
Copy link
Member

Choose a reason for hiding this comment

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

--compact as name and description suggests feature but in reality is introduced only to support backward compatibility. Names are hard but if we can add something more descriptive:

--legacy-output
--old-format

Etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure I'm happy to go with that

Copy link
Member

Choose a reason for hiding this comment

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

Chatted offline. Compact seems much better as we do not use old SDK and this flag can stay with us without removal

@wtrocki
Copy link
Member

wtrocki commented Jun 15, 2023

Awesome work. If the decision is to make those flags temporary we might add some notes to the code to remeber to remove them at specific moment of time

wtrocki
wtrocki previously approved these changes Jun 15, 2023
Copy link
Contributor

@sarahsimpers sarahsimpers left a comment

Choose a reason for hiding this comment

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

A couple thoughts on wording for the flag

Co-authored-by: Sarah Simpers <82042374+sarahsimpers@users.noreply.github.com>
Copy link
Contributor

@sarahsimpers sarahsimpers left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes!

@matt-condon matt-condon merged commit e7417ba into master Jun 15, 2023
@matt-condon matt-condon deleted the fix/compactListResults branch June 15, 2023 14:01
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.

4 participants