-
Notifications
You must be signed in to change notification settings - Fork 7
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
Comment about build size regression remains after its fixed #34
Comments
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" ***@***.***> wrote:
The last commit on angular/angular#22040
<angular/angular#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
<#34>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFnHeC5r6AV-kMNyQz5dl_b6V1vOaOuks5tSI9SgaJpZM4R7dCU>
.
|
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 <notifications@github.com>
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" ***@***.***> wrote:
> The last commit on angular/angular#22040
> <angular/angular#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
> <#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
<#34 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC5I7d_L6xL_Le2zqHJgIE8rdL1qZMsks5tSbG9gaJpZM4R7dCU>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The last commit on angular/angular#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
The text was updated successfully, but these errors were encountered: