johnmyleswhite / kNN.jl

The k-nearest neighbors algorithm in Julia
Other
22 stars 17 forks source link

Switch from deprecated Distance.jl to Distances.jl #16

Closed jeffreyrogers closed 9 years ago

jeffreyrogers commented 9 years ago

Distance.jl is deprecated now so it makes sense to switch to Distances.jl. (This is my first pull request to an open source project, so I hope I'm following the proper protocol here.)

johnmyleswhite commented 9 years ago

This looks great. Thanks!

Any chance you might squash your commits into one before merging?

jeffreyrogers commented 9 years ago

Hmm, I just tried to squash them, but I'm not sure how successful that was, since it looks like I've added a merge commit as well. I used git rebase -i HEAD~3. Is that how I should have done it?

johnmyleswhite commented 9 years ago

Assuming you've done your work on another branch, the easiest thing is git rebase -i master. Then squash all commits except the first. Then edit the commit messages so that only one is left.

jeffreyrogers commented 9 years ago

I was just doing the changes in my master branch (which I guess was a bad idea?). So any idea what I should do from here?

johnmyleswhite commented 9 years ago

Try git rebase -i origin/master?

-- John

On Oct 18, 2014, at 8:38 PM, Jeffrey Rogers notifications@github.com wrote:

I was just doing the changes in my master branch (which I guess was a bad idea?). So any idea what I should do from here?

— Reply to this email directly or view it on GitHub.

jeffreyrogers commented 9 years ago

Okay, looks like I fixed it (I ended up rebasing again and then doing git push -f to force the changes)

johnmyleswhite commented 9 years ago

LGTM

johnmyleswhite commented 9 years ago

Thanks for doing this. For future reference, developing in a non-master branch is a good practice since it makes it easier to update your code against the main repo in case the main repo changes out from under you.

jeffreyrogers commented 9 years ago

No problem, and I'll make sure to do that for next time.