Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Conversation

levi
Copy link
Contributor

@levi levi commented Oct 22, 2015

Went through @appleguy's post-merge review on #694 and cleaned up various aspects of the supplementary node implementation.

@appleguy
Copy link
Contributor

OK, I don't really know the correct workflow here for fixing a conflict for the author. I've tried a bunch of different things over time :). Please teach me if someone knows.

In this case I was able to do it, but "publishing" the rebase (even for the directly-checked-out PR) just created a new PR. Either way I'm going to try to land that one.

The conflict was from another PR that also set the =nil to =NO, which I hadn't realized this diff also did, because I reviewed it second.

@appleguy
Copy link
Contributor

the other PR is #771

@appleguy
Copy link
Contributor

Haha, ironically I had to revert the diff that conflicted with this one because it broke tests, and that caused my alternate PR to be the one that won't apply cleanly! Now we can land this one :).

Thanks for these refinements, I really like them!

appleguy added a commit that referenced this pull request Oct 26, 2015
Respond to supplementary node feedback
@appleguy appleguy merged commit 7d013a7 into facebookarchive:master Oct 26, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants