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

Clean up Example.elm #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ForgottenGensym
Copy link

There were a couple of problems in examples/Example.elm that I wanted to help out with:

  1. Not well commented, these kind of example programs are extremely helpful for learning the way things work, having both the code (example program) and documentation (comments) in one place and linked to each other is a powerful tool for understanding a new library. It's not a necessity, but in my opinion it can really help.
  2. The use of Debug.toString, which is not considered suitable for production use (afaik at least by the community). I wrote a routeToString function instead, which does add some boilerplate, but I think that the use of Debug.toString should be discouraged. On the other hand, the history tab is, in a way, debugging. I'm open to changing it or not, but I do think that Debug.toString should be, as previously mentioned, discouraged.
  3. A typo ("Uknown URL")

These are all things that aren't major issues, and a lot of it is somewhat opinionated on my behalf. I'm more than happy to drop some (or all) of them, but I really do think this could help out.

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.

1 participant