Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pluralization of 'Standort' / 'Standorten' #17

Merged
merged 1 commit into from Jun 11, 2018
Merged

Conversation

rnestler
Copy link
Contributor

Previously it showed "1 Standorten".

@rnestler rnestler requested a review from dbrgn June 10, 2018 21:08
@rnestler
Copy link
Contributor Author

I'm not sure if this fix is even worth it, since the wording in general dosn't make to much sense for only 1 sensor.

Copy link
Contributor

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

src/Views.elm Outdated
{-| View: Map
-}
mapView : Model -> Html Msg
mapView model =
page
("Finde die aktuelle und historische Wassertemperatur an "
++ (model.sensors |> List.length |> toString)
++ " Standorten rund um den Zürichsee!"
++ (pluralize " Standort" " Standorten" (model.sensors |> List.length)) ++ " rund um den Zürichsee!"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you break the line at ++? You should probably install elm-vim for auto-formatting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Alternatively you can manually reformat with elm-format on the command line.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I made this fix before I did the elm tutorial, because since then I have the plugin installed. Will fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@dbrgn
Copy link
Contributor

dbrgn commented Jun 10, 2018

I'm not sure if this fix is even worth it, since the wording in general dosn't make to much sense for only 1 sensor.

Best fix would be to have more sensors 🙂 Maybe you could ask at Sensirion?

@rnestler
Copy link
Contributor Author

@dhasenfratz Because you mentioned it today 😉

Copy link
Contributor

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terriffic!

@rnestler rnestler merged commit cbf350c into master Jun 11, 2018
@rnestler rnestler deleted the pluralization-fix branch June 11, 2018 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants