elm-explorations / test

Write unit and fuzz tests for Elm code.
https://package.elm-lang.org/packages/elm-explorations/test/latest
BSD 3-Clause "New" or "Revised" License
236 stars 40 forks source link

update compiler to 0.19.1 #145

Closed harrysarson closed 4 years ago

harrysarson commented 4 years ago

Upgrading requires a hack to work around compiler error when importing non-local kernel models.

I am not sure it is worth it.

harrysarson commented 4 years ago

@drathier can you purge the cache please :angel:

drathier commented 4 years ago

Done, and restarted

harrysarson commented 4 years ago

Test failure is legit, some sort of sub par version of sed on mac.

drathier commented 4 years ago

LMK when you want a review and/or merge

harrysarson commented 4 years ago

I would love a review! I think this is the best patch out there to upgrade the compiler to 0.19.1. However, tests/make.sh contains such a nasty collection of hacks that it may be better to keep using elm 0.19.0.

drathier commented 4 years ago

Do we expect to find/avoid any bugs/issues by moving to 0.19.1? It seems like a pretty hacky change. I haven't worked in this code base for a while, so I don't remember what we should be thinking about here.

harrysarson commented 4 years ago

There is not much difference between elm 0.19.0 and 0.19.1 so unlikely to make much difference. Keeping our dependencies up to date is my main motivation. It also seems like a bit of an issue if the elm test runner's test suite cannot pass with the most recent release of elm.

drathier commented 4 years ago

@rtfeldman @mgold any input?

rtfeldman commented 4 years ago

I don't have strong feelings about this one way or the other...one the one hand, it's not like we're getting a big benefit, but on the other hand, it's two commits, so easy to roll back if we regret it.

I'd say feel free to merge if you like, @harrysarson!

harrysarson commented 4 years ago

@drathier I can squash my commits before we merge this. (I only have commit rights on the test-runner repo not here)

drathier commented 4 years ago

Aight, imma merge this then :)

harrysarson commented 4 years ago

@drathier can you purge the cache please 👼

Thanks for merging! Please purge the cache for master! (Here is link if you would like to save some button clicking.)

drathier commented 4 years ago

Thanks for your help! Appreciate the link too :) Caches are nuked, builds restarted, and master is green!