refactor: Make workspace new use the views package for producing terminal output - #39191
Conversation
SarahFrench
added
the
no-changelog-needed
39cc9e7 to
24901e9
Compare
|
|
||
| c.Ui.Output(c.Colorize().Color(fmt.Sprintf( | ||
| strings.TrimSpace(envCreated), workspace))) | ||
| view.LogWorkspaceCreationSuccess(workspace, diags) |
There was a problem hiding this comment.
That's interesting that it outputs the success before we're actually done, not that we should change anything in this PR 😆
There was a problem hiding this comment.
I have mad deja vu about this, so maybe I have a forgotten branch kicking around to fix this. Ideally there'd be distinct success messages of 1) workspace made or 2) workspace made and populated with state, and then the ability to report a failure to the user where there was the side effect of an empty state being made but failing to populate it with data.
I'll look, but deffo beyond this PRs scope!
There was a problem hiding this comment.
Here we go! #39192
I'll rebase etc after this PR and then propose the changes. As this command only makes human output I believe we're free to update output freely like this.
Constants used for output are moved into the views package, so _for now_ the old use of `Ui` required importing that const that's been moved.
… to produce human output.
24901e9 to
51adef4
Compare
51adef4 to
dd77deb
Compare
…and use views methods to make assertions about output.
dd77deb to
cf77dff
Compare
|
Thanks @austinvalle ! |
2 participants
This PR refactors
workspace newuse theviewspackage for human output.There are some future changes I want to make in a separate PR that will ensure that all paths through the command result in only a single invocation of a view method, as this is necessary if JSON output will be added to this command in future. But this PR for now maintains the commands structure as-is.
Target Release
1.18.x
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry