Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify summary text. #2136

Merged
merged 2 commits into from Oct 31, 2018
Merged

Simplify summary text. #2136

merged 2 commits into from Oct 31, 2018

Conversation

CyrusNajmabadi
Copy link
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Oct 31, 2018

Fixes: #2051

Tweaks how we output summary text. Main points:

  1. Always one less line than before due to merging 'total changes. unchanged' info into one line.
  2. don't show total-changes if it is redundant with the changes we are showing. i.e. don't show 10 changes if we only are showing + 10 to create
  3. show the total summary after the individual pieces. i think this just makes more sense since it's the sum of all the little operations we're making.

Multiple changes:

image

One type of change:

image

No changes:

image

All changes:

image

Copy link
Contributor

@chrsmith chrsmith left a comment

Choose a reason for hiding this comment

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

It looks like this is fine, but could you double check the new formatting won't break the unfortunate hack we currently have in the service for grepping the output? Specifically, that we still have have whitespace in front of every line that lists a kind of resource change, and then a blank line at the very end. (Once we send the actual engine events to the service we won't need to rely on that approach...)

https://github.com/pulumi/pulumi-service/blob/master/cmd/service/services/cloud/util.go#L97
https://github.com/pulumi/pulumi-service/blob/master/cmd/service/services/cloud/util_test.go#L36

@CyrusNajmabadi
Copy link
Contributor Author

Specifically, that we still have have whitespace in front of every line that lists a kind of resource change, and then a blank line at the very end.

Yup. That part should be fine. There should still always be at least one line. And it's still lines that start with 4 spaces, and a final blank line.

Note: those test files will likely be affected here if they are actually validating real output. If they're just validating that you can strip color codes though, it should be ok.

@CyrusNajmabadi CyrusNajmabadi merged commit 76d08f8 into master Oct 31, 2018
@pulumi-bot pulumi-bot deleted the summary branch October 31, 2018 04:57
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.

None yet

2 participants