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

Edit PR comment option overrides another stack's comment if it's prefix of another #1165

Open
Xander-Polishchuk opened this issue May 13, 2024 · 2 comments
Labels
kind/bug Some behavior is incorrect or out of spec

Comments

@Xander-Polishchuk
Copy link

Xander-Polishchuk commented May 13, 2024

What happened?

The code responsible for updating PR comments, checks header with startsWith, which prone to override another stack's comment if one stack is named as prefix of another if run after.

In our case we have two stacks, default region dev stack and stack for some specific region, e.g. dev and dev-tokyo

The issue code line:

actions/src/libs/pr.ts

Lines 56 to 58 in 0806784

const comment = comments.find((comment) =>
comment.body.startsWith(heading) && comment.body.includes(summary),
);

A simple, quick solution is to add some ending symbol like . after ${stackName}:

const heading = `#### :tropical_drink: \`${command}\` on ${projectName}/${stackName}`;

Like: #### :tropical_drink: \`${command}\` on ${projectName}/${stackName}.

Example

Create stack with name dev and another stack with name dev-tokyo.

Output of pulumi about

N/A

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@Xander-Polishchuk Xander-Polishchuk added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels May 13, 2024
@justinvp justinvp removed the needs-triage Needs attention from the triage team label May 14, 2024
@justinvp
Copy link
Member

Thanks for opening the issue and the suggestion.

I'm curious if we could look for newline characters at the end of the heading (\n or \r\n) rather than introducing the . at the end of the heading.

Would you be interested in contributing a fix, @Xander-Polishchuk?

@Xander-Polishchuk
Copy link
Author

I would say any solution that checks ${stackName} as a whole instead of prefix will work. So, \n / \r\n should work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec
Projects
None yet
Development

No branches or pull requests

2 participants