Comment about build size regression remains after its fixed

[alexeagle]

The last commit on https://github.com/angular/angular/pull/22040 fixes the size regression, as indicated in the status

However, the comment from buildsize is still on the thread and was not updated or removed

[Daniel15]

Oh, interesting. I think it's because I added logic to not comment if there's no regression, but it doesn't handle the case where a comment already exists and needs to be updated.

Do you think it should delete the comment, or should it just edit it to say that the regression was fixed?

Sent from my phone

On Feb 6, 2018 1:43 PM, "Alex Eagle" [email protected] wrote:

The last commit on angular/angular#22040 https://github.com/angular/angular/pull/22040 fixes the size regression, as indicated in the status [image: el4s0djjazw] https://user-images.githubusercontent.com/47395/35874717-217697be-0b22-11e8-9a85-acc1c16215d1.png

However, the comment from buildsize is still on the thread and was not updated or removed [image: 5c2mfy9pu4t] https://user-images.githubusercontent.com/47395/35874765-37bbf6cc-0b22-11e8-9a19-6c70412e9e22.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Daniel15/BuildSize/issues/34, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFnHeC5r6AV-kMNyQz5dl_b6V1vOaOuks5tSI9SgaJpZM4R7dCU .

[alexeagle]

I think it's better to update the comment. A reviewer who saw the previous comment needs to know it's fixed, otherwise they just wonder what happened to the comment (and I think revising history always violates principle of least surprise)

On Wed, Feb 7, 2018, 6:22 AM Daniel Lo Nigro [email protected] wrote:

Oh, interesting. I think it's because I added logic to not comment if there's no regression, but it doesn't handle the case where a comment already exists and needs to be updated.

Do you think it should delete the comment, or should it just edit it to say that the regression was fixed?

Sent from my phone

On Feb 6, 2018 1:43 PM, "Alex Eagle" [email protected] wrote:

The last commit on angular/angular#22040 https://github.com/angular/angular/pull/22040 fixes the size regression, as indicated in the status [image: el4s0djjazw] < https://user-images.githubusercontent.com/47395/35874717-217697be-0b22-11e8-9a85-acc1c16215d1.png

However, the comment from buildsize is still on the thread and was not updated or removed [image: 5c2mfy9pu4t] < https://user-images.githubusercontent.com/47395/35874765-37bbf6cc-0b22-11e8-9a19-6c70412e9e22.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Daniel15/BuildSize/issues/34, or mute the thread < https://github.com/notifications/unsubscribe-auth/AAFnHeC5r6AV-kMNyQz5dl_b6V1vOaOuks5tSI9SgaJpZM4R7dCU

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Daniel15/BuildSize/issues/34#issuecomment-363783764, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5I7d_L6xL_Le2zqHJgIE8rdL1qZMsks5tSbG9gaJpZM4R7dCU .