Hacker News new | ask | show | jobs
by js2 5309 days ago
[Edited for tone which I guess is the reason for the down votes. I appreciate the quick response below.]

Most of this patch is adding libuv, which is included in its entirety due to "the version included in the patch is different than the one available on github, some changes have been added to the code". There is also a lot of cleanup. Also a small nit, in the patch instructions:

  git checkout 3fac86ff1d
  git checkout -b 2.4_win_uv
is equivalent to:

  git checkout -b 2.4_win_uv 3fac86ff1d
Here's what would make it more easily reviewable.

1. clone redis

2. clone libuv

3. make whatever changes needed to libuv as its own commit.

4. add libuv as a submodule to redis.

5. perform all the misc compiler cleanup stuff to redis as its own commit; usually you want your cleanup/refactored to happen before you perform functional changes.

6. add the ms-specific code as its own commit.

7. push up the new commits to the two forked repos.

This would make it all a bit easier to review. The only questionable part is adding libuv as a reddis submodule (4). Maybe I'd leave that part out initially and instead just specify the equivalent manual step needed there (clone our fork of libuv into X and checkout Y).

2 comments

Excellent. Thank you, and I apologize for the harshness of my original comment. Was honestly just trying to be helpful.

  > 1. cloned redis
  > 2. cloned libuv
Of course you mean "fork", not "clone".