zotero / citeproc-js-server

Web service to generate citations and bibliographies using citeproc-js
Other
60 stars 39 forks source link

Latest version of Node.js, citeproc-js, csl, csl-locale; use npmlog for logging; brushing and polishing. #12

Closed Klortho closed 10 years ago

Klortho commented 10 years ago

Here is the pull request I promised.

Summary of the changes:

dstillman commented 10 years ago

Thanks for working on this. @fcheslack will review this, but please remove the tabs -> spaces commit.

dstillman commented 10 years ago

Thanks, but can you rebase and remove the original commit?

Klortho commented 10 years ago

I'm not sure I did it right -- I've never used rebase before. I did rebase -i HEAD~15, and removed the two commits: tabs -> spaces, and the other commit where I reverted tabs->spaces. I see that in csl_nodejs.js, now, I'm only "blamed" for one line, so I think it's right. I'll test the code now to make sure it still runs.

Klortho commented 10 years ago

Yes, it still works. And I looked at the graph in gitk, and it looks correct.

dstillman commented 10 years ago

Sorry, still not correct. Just look above in this pull request — you can see that the spaces commits are still there. (Also, a bunch of commits with the same message are separated across multiple commits and should be squashed together, some commits are duplicated, there's an extra merge commit that shouldn't be there, and generally there's just a lot of your internal work that doesn't belong in the public timeline.)

Note that you have to force-push back to this branch after rebasing (and git should require you do so, so if it's not, you're likely doing something else wrong).

dstillman commented 10 years ago

Also, no need to specify a commit in the rebase for this. You should just need to do git rebase -i upstream/master (assuming upstream is what you named the upstream branch), and it will show you all the commits you've added.

Klortho commented 10 years ago

Let me know if it's okay now. I created a new branch from the first commit before I started to do any work, then cherry-picked all the commits of mine (one copy each) except the "spaces" ones, and then rebased master from that. Thanks for your patience -- still learning!

dstillman commented 10 years ago

Better. Still a few things that should be trivial to fix, though. No need to use cherry-pick. Just do git rebase -i upstream/master (again, assuming zotero/citeproc-node is upstream, which you can see with git remote -v. You should see a list of all of these commits in chronological order. For the few that are split across multiple commits with the same message (e.g., "Adding more comments") or that might as well be combined ("Brushing"/"more brushing"), edit the second line so that it begins with 'fixup' instead of 'pick', which will cause that commit to be squashed into the one above with the second one's commit message discarded. Then force push.

You can do more complicated things with rebase — rearranging commits, editing commits — but those can get trickier. This should be super easy, though.

Klortho commented 10 years ago

Has anybody had a chance to look at this? I think I made some significant improvements. (But, of course, I am biased. ;) )