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

NOTICE: master is incorrect atm #18403

Closed
Fishrock123 opened this issue Jan 26, 2018 · 7 comments
Closed

NOTICE: master is incorrect atm #18403

Fishrock123 opened this issue Jan 26, 2018 · 7 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@Fishrock123
Copy link
Member

Looks like @maclover7 had some trouble landing something and commits back to 64ed366e6fc62af434b28c8b3ebd815e6ba51c69 are not correct atm.

I was in irc with him helping him resolve the land but had to step away and I'm not sure what happened. He doesn't seem to be around anymore.

I have a correct copy up to the commits I landed today (what would supposedly be HEAD atm) I'm going to force push in a few minutes if I don't get a response back from @maclover7 or someone objects.

@apapirovski
Copy link
Member

apapirovski commented Jan 26, 2018

@Fishrock123 Please just force push immediately whatever your latest snapshot is. This looks badly broken. I don't think we want someone continuing to land on top of this already broken state.

@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Jan 26, 2018
@Fishrock123
Copy link
Member Author

Fwiw the commits in question are 2cb9e2a...993b716

@cjihrig
Copy link
Contributor

cjihrig commented Jan 26, 2018

I just force pushed master. Current head is now 082f952.

@Fishrock123
Copy link
Member Author

I re-added the remaining commits. Current head is now https://github.com/nodejs/node/commits/bb5575aa75fd3071724d5eccde39a3041e1af57a

We are OK now so I'm going to close this.

@maclover7 please reach out if you need assistance. :)

@jasnell
Copy link
Member

jasnell commented Jan 26, 2018

Thank you for sorting it out.

@maclover7
Copy link
Contributor

I apologize -- this originally started when I was trying to land 18263, and the patch was not applying properly. I decided I would try and merge the PR, and then amend the commit to add the metadata. @Fishrock123 merged his timers PRs at the same time, so there was a conflict. I then amended 18263's commit and added the metadata, and pushed to upstream, but it looks like that made me the committer on all commits from 18263's commit to HEAD.

It turns out, in hindsight, that the change from 18263 had already been applied via 17407 (if you look the same exact commit is there), so 18263 became irrelevant when 17407 was merged. I am sorry for all of the hassle.

@joyeecheung
Copy link
Member

joyeecheung commented Jan 27, 2018

@maclover7 Just curious, is this what happened?

Fishrock123> git am the timer PR
Fishrock123> git push to master 

maclover7> git fetch 18263 to a local branch
maclover7> git checkout master
maclover7> git fetch master and update it to where Fishrock123 has pushed
maclover7> git merge 18263
maclover7> ??? something happened that resulted in 2cb9e2a...993b716
maclover7> git commit --amend the message
maclover7> git push to master # where is started to be broken with extra commits

cjihrig > git push --force # to 082f952

Fishrock123> git cherry-pick  082f952...bb5575a
Fishrock123> git push to master # back to normal

I wonder wether a pre-push git hook could help with this...see nodejs/node-core-utils#160 but I am not sure how to reproduce those extra commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

7 participants