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

Revert da4a4ac and add more longDesc #3164

Closed
wants to merge 1 commit into from
Closed

Conversation

RichardLitt
Copy link
Member

This reverts most of the changes in da4a4ac, which were later found to be errorful. See #2657 for details.

License: MIT
Signed-off-by: Richard Littauer [email protected]

This reverts most of the changes in da4a4ac, which were later found to be errorful. See #2657 for details.

License: MIT
Signed-off-by: Richard Littauer <[email protected]>
@RichardLitt RichardLitt added need/review Needs a review status/in-progress In progress labels Sep 1, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Sep 1, 2016

Can you use revert commit for the da4a4ac changes and thus separate new changes?

@RichardLitt
Copy link
Member Author

@Kubuxu I went through manually and looked at the diffs for the new ones. Would revert have done that automatically? I still think this should work.

@Kubuxu
Copy link
Member

Kubuxu commented Sep 1, 2016

git revert da4a4ac would create commit that reverts the da4a4ac commit in full.

@RichardLitt
Copy link
Member Author

This was more subtle, as I added some stuff.

@Kubuxu
Copy link
Member

Kubuxu commented Sep 1, 2016

Yeah, but in that case you can create revert commit and commit that adds the changes.

@RichardLitt
Copy link
Member Author

RichardLitt commented Sep 1, 2016

@Kubuxu Should I close this and reopen with that?

@Kubuxu
Copy link
Member

Kubuxu commented Sep 1, 2016

It is @jbenet call, as the PR is in response for his CR.

@jbenet
Copy link
Member

jbenet commented Sep 9, 2016

@RichardLitt it will make it easier to understand the change if you do a revert commit and then your changes on top. pls thanks.

no need to close and reopen, you can just git push origin feat/undo-a4a4ac -f

@jbenet
Copy link
Member

jbenet commented Sep 9, 2016

Ask for git help if you get stuck rebasing

@RichardLitt
Copy link
Member Author

I might need some git help, then.

15:24 ~/src/ipfs/go-ipfs (feat/revert-da4a4ac) 🐕  git revert da4a4ac0bc26b80457f537b950c5e43130bce242
error: Commit da4a4ac0bc26b80457f537b950c5e43130bce242 is a merge but no -m option was given.
fatal: revert failed
15:25 ~/src/ipfs/go-ipfs (feat/revert-da4a4ac) 🐕  git revert da4a4ac0bc26b80457f537b950c5e43130bce242 -m 2
On branch feat/revert-da4a4ac
nothing to commit, working directory clean

Using git revert doesn't seem to do anything. I could manually try and revert the two parent commits?

@Kubuxu
Copy link
Member

Kubuxu commented Sep 11, 2016

The mainline -m is 1 in this case.
You have to also remove existing changes from the branch, you can use git reset --hard origin/master for that, warning it will remove the changes you made.

Also it looks like revert in this cases gives quite an ugly looking revert conflict but it isn't that bad really.

@RichardLitt
Copy link
Member Author

Interesting! How do you know that it -m is 1? The PR has two commits:

screen shot 2016-09-11 at 9 33 44 am

@ghost
Copy link

ghost commented Sep 11, 2016

Interesting! How do you know that it -m is 1? The PR has two commits:

Numbering starts at 1, ordering is mainline -> merged commit, and mainline is the commit on top of which the merge was made :)

@Kubuxu
Copy link
Member

Kubuxu commented Sep 11, 2016

The commit you are reverting was created when the PR was merged, it wasn't the part of original PR. When it was merged two commits were joined into one chain, commit being HEAD of the master and commit being HEAD of your PR.

As you are reverting merge commit git asks you which commit chain that was merged in this merge commit is the "main" one, so it knows which commits chains from that merge to revert.

@RichardLitt
Copy link
Member Author

Closing in favor o #3239. Will use as reference for that.

@RichardLitt RichardLitt removed the status/in-progress In progress label Sep 19, 2016
@Kubuxu Kubuxu deleted the feat/undo-a4a4ac branch February 27, 2017 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants