search

exercism / elm-analyzer

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

Add analyzer rule for `bettys-bike-shop` #31

Closed jiegillet closed 2 years ago

jiegillet commented 2 years ago

Closes #17. Sister PR here.

The design.md document mentions two things:

This PR covers the first one, but I don't know how to address the second one (which is also mentioned for a couple of other concept exercises). Formatting in Elm is so reliant on elm-format that I don't think it makes sense to ask students to emulate that. There is an open issue that talks about adding a format button on the web editor. I'm not sure how far that had gone, but if we could have an elm-format button, that would solve everything.

jiegillet commented 2 years ago

Oh, I have another question. One of the comment in the stub file for the exercise is 

-- TODO: import the String module

But there is no need to import String since it's already imported by default. I was actually thinking of implementing an elm-review rule about that. Is this worth a PR for removing it, or ignoring is fine?

ceddlyburge commented 2 years ago

But there is no need to import String since it's already imported by default. I was actually thinking of implementing an elm-review rule about that. Is this worth a PR for removing it, or ignoring is fine?

I think it is probably best to remove this. We should just check that the exercise still achieves its goals though.

jiegillet commented 2 years ago

OK, I've added the new rules (and the new comment over here).

jiegillet commented 2 years ago

@ceddlyburge could you have another look at this? We updated the exercise, then I adapted the analyzer. Please also have a look at the comments.