search

informatikr / hedis

A Redis client library for Haskell.
http://hackage.haskell.org/package/hedis
BSD 3-Clause "New" or "Revised" License
330 stars 125 forks source link

Geospatial commands #115

Open kamyar1979 opened 5 years ago

kamyar1979 commented 5 years ago

Would you add geospatial commands? Can I help?

k-bx commented 5 years ago

Happy to review and merge a PR implementing them. Would you be willing to do one?

kamyar1979 commented 5 years ago

Sure! I would be happy to contribute!

kamyar1979 commented 5 years ago

PLease help me create a good PR! Should I fork the project first?

k-bx commented 5 years ago

@kamyar1979 yeah, I see the PR is a bit messed up because you had older fork probably. If your Git skills aren't very high -- I suggest completely deleting your fork first (go to kamyar1979/hedis repo page, and find "delete this repository" in settings), then fork this hedis repo, do your changes, commit and make a PR. This way it will be done on top of the latest commits.

kamyar1979 commented 5 years ago

I almost did the job. Still there is some remaining parts in georadius for implementing options, and also tests are not done. Today I will push a primary release.

kamyar1979 commented 5 years ago

Pushed first release. Please help mw for adding options, I am confused. I guess I need a instance implementation of encode for GeoRadiusOpts but I am not sure. Please check my fork and help me make it better! Also I have not yet added tests.

kamyar1979 commented 5 years ago

Sorry! I also did some minor fix and housekeeping in your code!

k-bx commented 5 years ago

@kamyar1979 where did you push it? You need to close this PR https://github.com/informatikr/hedis/pull/116 and open a new one which will only have your new commits present

kamyar1979 commented 5 years ago

I did it in my fork. I need you to check the code and then let me go on work before asking to merge.

kamyar1979 commented 5 years ago

I create a PR from my master to work-in-progress. I cant create a branch. Should I?

k-bx commented 5 years ago

@kamyar1979 please make a new branch that starts from the fresh master, once you will do everything correctly you should only see your commits in PR's commit list, not these >100 items seen in https://github.com/informatikr/hedis/pull/117

Probably need to do something like

git remote add upstream git@github.com:informatikr/hedis.git
git fetch remote
git rebase remote/master
git push -f origin

if you don't feel comfortable doing this – please see my earlier suggestion on completely deleting the repo and re-forking (but be sure to backup the changes to reapply them)

kamyar1979 commented 5 years ago

My fork is new! I forked the repo just after your first comment!