Skip to content

Remove ansi control chars from PR comment message#1231

Merged
tgummerer merged 6 commits into
pulumi:mainfrom
jmrt47:feat/colorize-pr-comments
Jul 24, 2024
Merged

Remove ansi control chars from PR comment message#1231
tgummerer merged 6 commits into
pulumi:mainfrom
jmrt47:feat/colorize-pr-comments

Conversation

@jmrt47

@jmrt47 jmrt47 commented Jul 18, 2024

Copy link
Copy Markdown
Contributor

Pulumi action used with color: always output and comment-on-pr: true lead to ANSI control characters in the Pull Request comment. Reported in #1229 check for details.

Special thanks to @bennettp123 who had the idea, converting the output from ansi to html and add html to the comment. I picked up the idea propsed in #859.

Changes proposed in this PR:

  • Convert output from ansi to html and add html as a comment
  • Add config to specifiy the maximum length of the PR comment output

@jmrt47 jmrt47 marked this pull request as ready for review July 18, 2024 20:04
@jmrt47 jmrt47 requested a review from a team as a code owner July 18, 2024 20:04

@tgummerer tgummerer left a comment

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.

Thanks for getting this started! I left a few comments inline.

Comment thread src/libs/__tests__/pr.test.ts Outdated
Comment thread src/libs/pr.ts
Comment thread README.md Outdated
Comment thread src/libs/pr.ts
commentMaxCharacter,
} = config;

// GitHub limits comment characters to 65535, use lower max to keep buffer for variable values

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.

❤️ thanks for documenting this!

@tgummerer tgummerer merged commit 4cda3e9 into pulumi:main Jul 24, 2024
@pulumi-bot

Copy link
Copy Markdown

This PR has been shipped in release v5.4.0.

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.

3 participants