-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Merge commits on master #19854
Comments
IMO it should be fixed. This wouldn't be the first time we break the 10 min rule. |
@nodejs/tsc ... how do we want to handle this? We should likely do a revert of those commits at this point. I would say that it's too late to force push. |
Possible ref: #19471 |
SGTM, let’s do it asap
Il giorno ven 6 apr 2018 alle 18:37 Anatoli Papirovski <
notifications@github.com> ha scritto:
… IMO it should be fixed. This wouldn't be the first time we break the 10
min rule.
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#19854 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADL49Vcukpvr2-3zZdWrXtTe1FRz3_tks5tl5nMgaJpZM4TKXCc>
.
|
I'm fine with a force push if other @nodejs/tsc are. |
+1 on force pushing. Otherwise we have a weird commit history. |
+1 on force push |
+1 |
As an aside I've sent a feature request to github about finding a way to block merge commits from landing on branches |
+1 as well, |
I've force-pushed, removing the relevant commits. |
ref: #19471 (comment) |
There are two merge commits on master:
I don't know exactly when they were pushed and if it's something that we need to absolutely fix, but something else already landed on top of those.
/cc @nodejs/tsc @gireeshpunathil
The text was updated successfully, but these errors were encountered: