Trim PR comments from the front.

[devonmoss]

I believe that the resources summary is the most important component of the output provided by pulumi preview and should always be included in the pull request comment. When trimming from the back, long outputs will not contain this information, which requires navigating to the checks tab and looking at the output there.

Perhaps this is just a reflection of how I use pulumi but don't think so because we are all conditioned by the local use of the pulumi cli. When we run pulumi preview locally the output ends up right at the bottom so the first thing we see is the summary something like this:

Resources:
      ~ 85 to update
      +-26 to replace
      111 changes. 1062 unchanged

In many cases this is all the information that is needed to confirm that I'm ready to run pulumi up. Other times I need to look more closely at the details for each resource but even still I have the information from that summary to give me an idea of what I'm looking at as I scroll back through the output.

This change trims the PR comment from the front if it is ever so long that it must be trimmed.

[tgummerer]

Thanks for the contribution. I'm not sure I agree that this is better than what we have currently though. Could you give us a bit of a description of why you think this is better? (This should also be included in the PR description) Maybe we could make this behaviour optional and add another flag to toggle it? Trimming from the front of the message seems unconventional and should probably not be the default behaviour.

[devonmoss]

I've updated the description for my PR.

I agree that generally speaking, trimming from the front is unconventional but in this case it causes a scenario where the resources summary is not shown in the pull request comment and I can't see why this would ever be a desirable situation.

[voldemortensen]

I agree with @devonmoss here. When I'm reviewing potential changes, I am much less interested in the output of the pip install and much more interested in which resources are going to be effected by a merge request. In fact, unless there's an error with pip install, I'm not really interested in that output at all.

[devonmoss]

I've added a commit which allows this to be configured optionally. My apologies for not reading the CONTRIBUTING.md file before submitting. I see now that I should have submitted an issue first. I'll be better next time.

[tgummerer]

I've added a commit which allows this to be configured optionally.

Thanks, this is great! I think we should be able to flip this to be the default in the next major release, still giving the users the option to opt-out. I'm mainly wary of changing this in a minor release.

My apologies for not reading the CONTRIBUTING.md file before submitting. I see now that I should have submitted an issue first. I'll be better next time.

No worries. It helps sometimes to discuss the details first to avoid some roundtrips in PRs, but we're always happy to review PRs. I left a couple of minor comments inline, once they are addressed I think we can get this merged!

[devonmoss]

This is the reason why I added an extra new line. Previously, when the output was trimmed it would add a warning message to the end of the comment. My change makes it so in the event that we trim from the front it will add the warning message near the beginning of the comment, right before the trimmed output.

The blank line above was not there during the first pass I made and the tests passed without the addition of the extra \n to the tests. However when I used it in a project the markdown doesn't format properly. Instead of:

⚠️ Warn: The output was too long and trimmed from the front.

It did:

⚠️ **Warn**: The output was too long and trimmed from the front.

[tgummerer]

Thanks for implementing this!

[pulumi-bot]

This PR has been shipped in release v5.5.0.