search

exercism / elm-analyzer

GNU Affero General Public License v3.0
1 stars 4 forks source link

Write analysis for concept exercise `blorkemon-cards` #44

Closed jiegillet closed 2 years ago

jiegillet commented 2 years ago

Closes #42.

I didn't follow exactly the design document, I added one to check that maxPower uses max (to my surprise, someone didn't) and I removed sortByCoolness uses List.sortBy because the hints also present List.sortWith as a possibility.

I'm too sleepy for doing the comment PR, so that will have to wait for tomorrow, I think. Sister PR here.

The CI broke because the brand new version of elm-test requires elm-explorations/test 2.0, so I fixed it for now, I will do a dependency update later.

mpizenberg commented 2 years ago

The CI broke because the brand new version of elm-test requires elm-explorations/test 2.0, so I fixed it for now, I will do a dependency update later.

Regarding this, it would be nice if you could add an elm-tooling.json as in the elm repo https://github.com/exercism/elm/blob/main/elm-tooling.json And with that, you can use the elm-tooling-action github action that installs the specified tools much more efficiently than npm. (cf https://github.com/exercism/elm/blob/main/.github/workflows/build.yml#L25) Benefits is you know exactly the version of the tools used both locally and in CI. Elm-test is not available via elm-tooling though so you still need to install it via npm, or switch to elm-test-rs which can be installed via elm-tooling.

jiegillet commented 2 years ago

Yep, sounds good. I seem to remember elm-test-rs didn't work here for some reason but I'll investigate. Probably a different PR, if it's OK I'll ping you to review it!

jiegillet commented 2 years ago

Thank you for the review @ceddlyburge

@exercism/maintainers-admin can I get an approval please? 💛