lost-pixel / lost-pixel

Open source alternative to Percy, Chromatic, Applitools.
https://lost-pixel.com/
MIT License
1.35k stars 46 forks source link

Github actions: update comment rather than re-create #270

Open Betree opened 1 year ago

Betree commented 1 year ago

Mode

Other

Feature description

Use case

Lost Pixel currently posts a new comment for each force push, which creates some noise for projects that extensively use Git in this way. Some comments could event get lost as GitHub collapses activities when there are too many.

I would love to have a single Lost Pixel comment at the top that gets updated based on CI status.

Screenshots

![image](https://github.com/lost-pixel/lost-pixel/assets/1556356/71491701-4993-4505-825f-294ea2d7012a) ![image](https://github.com/lost-pixel/lost-pixel/assets/1556356/005d7d6c-cc7c-49b7-a3c6-ce3fb9a72674)

What are other tools doing?

Vercel deploy preview

Codecov

Lets you configure between updating (default) or deleting and then re-creating: https://docs.codecov.com/docs/pull-request-comments#behavior.

d-ivashchuk commented 1 year ago

This is actually pretty awesome idea to update single comment instead of always posting a new one! Should be possible 👍🏼

Lost Pixel is really not having the best synergy with force pushes as we calculate the baselines in the git history 🥲 do you think this flow is still usable all together?

To clarify - within single PR you might need to update baselines numerous times until you merge if you choose so, usually if you change one thing visually approve it and then commit unrelated to visual changes in your pr lost pixel will pass, in your case commit history gets distorted and you need to approve after every commit.

Betree commented 1 year ago

Our need is to make sure that there's no regression between the pull request and main; we don't care that much about diffs between individual commits on the same PR. The fact that we'll have to re-approve the diffs is not ideal, but not blocking either - we can definitely live with it.

Out of curiosity, I just looked at how Vercel does that and it looks like they're adding a hidden token in the comment to identify it (I suppose vc stands for Vercel Comment):

image

d-ivashchuk commented 1 year ago

I wish there was a nicer way on our platform to work with force pushes :( you did not have this issue with chromatic, right?

I did not fully get the meaning of hidden comment 🙂 you mean they use it to identify what to update when searching trough issue comments?

Betree commented 1 year ago

you did not have this issue with chromatic, right?

The CI logs are expired, so I can't double-check, but from what I remember chromatic used to remember validated diffs even after a force push.

you mean they use it to identify what to update when searching trough issue comments?

Correct, I suppose that they're doing that.

d-ivashchuk commented 1 year ago

We will try to think how to avoid the duplicated approvals on force pushes as well. Thanks for the idea of improving the comments!

Betree commented 1 year ago

As a workaround, a setting to turn off comments on PRs would be great. On PRs with multiple force pushes, we often end up with dozens of comments that blotter the thread, while the CI job status would be just enough.

d-ivashchuk commented 1 year ago

@Betree we currently don't have an option on the platform to just disable the gh integration :/ miss on our side, would it help if I disable it for you unit we have a better way of handling this(via updating the comment as agreed)?

Betree commented 1 year ago

@Betree we currently don't have an option on the platform to just disable the gh integration :/ miss on our side, would it help if I disable it for you unit we have a better way of handling this(via updating the comment as agreed)?

Ideally, we would just disable the Github comments (while keeping the CI job). If not feasible then yes, disabling the Github integration altogether would work for us. Thanks!

d-ivashchuk commented 1 year ago

of course GH integration would continue to work, it's misleading on the platform, what you've enabled is purely for posting comments!

d-ivashchuk commented 1 year ago

Done, GH comments are gone, check is still present!