zellige / hs-geojson

GeoJSON parsing library [Haskell]
BSD 3-Clause "New" or "Revised" License
23 stars 13 forks source link

Remove doctests and add support for Numeric/String FeatureID #15

Closed ozmat closed 6 years ago

ozmat commented 6 years ago
domdere commented 6 years ago

Cool, thanks, I'll review ASAP

ozmat commented 6 years ago

Thanks !

Yes I was going to suggest you to update your CI file and use stack to build/test, something like :

pre:
  - wget https://github.com/commercialhaskell/stack/releases/download/v1.6.1/stack-1.6.1-linux-x86_64.tar.gz -O /tmp/stack.tar.gz
  - mkdir /tmp/stack/
  - tar -xvzf /tmp/stack.tar.gz -C /tmp/stack/
  - export STACK=/tmp/stack/stack-1.6.1-linux-x86_64/stack

setup:
  - $STACK setup --no-terminal
  # for dependencies, will fail faster if there is a problem
  - $STACK build --no-terminal --only-configure

compile:
  - $STACK build --no-terminal

test:
  - $STACK test --no-terminal

If you want to have faster builds (I saw that you were checking for a system GHC in your current CI file) you can try this one :

$STACK build --system-ghc

which will actually check if the system version is compatible with the project configuration (particularly the resolver setting).

Otherwise you can use cache directories and only download GHC once (or each time the version/resolver changes).

- cache_directories:
  - "~/.stack"

You can probably cache your "./.stack-work" and avoid rebuilding dependencies if you really care about the build time (not sure about this one though).

domdere commented 6 years ago

hmm ok I was hoping to keep the stack dependence to a minimum unfortunately.

I actually regret adding a stack.yaml file to the project at all, I can fix up the CI in a seperate pass myself.

It could do with an update to make it more efficient, so I may as well do that at the same time.

ozmat commented 6 years ago

No It wasn't a mistake. We were actually using the original one (or maybe a fork that was using a newer lts), and needed support for the numeric FeatureID so we just implemented it a month ago, and I spent some time yesterday converting the test because It wasn't working with doctests (and doctests-discover).

domdere commented 6 years ago

I'll probably just update it to use my current standard CI yaml

domdere commented 6 years ago

ok, thanks @Ozmat I'll merge this in and do a pass to update the CI build

domdere commented 6 years ago

It might take a while for this to hit Hackage though

domdere commented 6 years ago

(Apologies btw, this project doesnt get much love from me)

domdere commented 6 years ago

@Ozmat If sitewisely want to take it over btw let me know.

Otherwise, the "half in, half out" approach with stack that I would take will look a little inconsistent, thats how much I despise stack unfortunately (I dont use it in my other projects at all).

ozmat commented 6 years ago

Sounds good, thanks ! No worries, we're using our fork for now and added some extra love for you ;)

Yep sure ! You can just give the collaborator access to @newmana and @Ozmat, it should be alright.

domdere commented 6 years ago

Ok added you two as collaborators, I'm also fine if sitewisely wants to get rid of their fork and I can transfer ownership of the repo to that organisation, so you aren't beholden to me in any way

(can also sort out the hackage maintainer privs)

newmana commented 6 years ago

Wasn't sure the best way to contact you - but we've finally gotten around to getting rid of our fork and we'd be happy to become the hackage maintainer.