search

geocaml / ocaml-rtree

A pure OCaml implementation of R-Trees
BSD 3-Clause "New" or "Revised" License
26 stars 7 forks source link

Refactor test code to not repeat Rtree code #25

Closed patricoferris closed 11 months ago

patricoferris commented 1 year ago

We repeat the following a lot

https://github.com/geocaml/ocaml-rtree/blob/7e00275e9ab4d2303b415d1f5a1baea1f38d2f6d/test/basic.ml#L162-L178

it would be good to factor this outside of the individual tests where possible.

khushishikhu commented 1 year ago

Hi @patricoferris I am willing to take up this issue. Please grant me the permission

khushishikhu commented 1 year ago

@patricoferris could please guide me a bit, that do I need to test it. and How will I test. According to the Project contribution information of ocaml, I have installed all the prerequisites but I just don't know I to start ocaml. Please help me out in this

It's showing me this "[ERROR] mdx = 2.2.0: not available because the package is pinned to version 2.1.0" whenever I try to run "opam install . --deps-only --with-test"

patricoferris commented 12 months ago

Hmm that's strange, I wonder how MDX got pinned? Can you try opam pin remove mdx and then installing the dependencies?

patricoferris commented 12 months ago

Any luck with this suggestion @khushishikhu ?

AryanGodara commented 11 months ago

@khushishikhu are you still working on this?

khushishikhu commented 11 months ago

@AryanGodara No

AryanGodara commented 11 months ago

@patricoferris Look like this can now be assigned to a new applicant

AryanGodara commented 11 months ago

@harshey1103 maybe you can give this a try. In the meantime we'll figure out how to assign the medium/enhancement issues

Mankavelda commented 11 months ago

Hello @patricoferris , can I work on this? @harshey1103 are you on it already?

AryanGodara commented 11 months ago

@Mankavelda There's already a merged PR for this issue. I think we should close it now. Unless you have a suggestion on how to further refactor the test code? :)

Mankavelda commented 11 months ago

@AryanGodara it's ok if there issue has already been dealt with. I will just do more research and if I can refactor the test or anything else, an issue can always be raised. For now I think you can close it

harshey1103 commented 11 months ago

Hi @patricoferris @AryanGodara, I think there is no more work left to be done on this issue. Can someone close it? Just concerned about my application as I worked on this issue :p

patricoferris commented 11 months ago

Fixed in https://github.com/geocaml/ocaml-rtree/pull/32