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

Improve regex to extract the city names from the filename #29

Merged
merged 1 commit into from
Apr 4, 2016

Conversation

dirkschumacher
Copy link
Member

This handles the "neu-ulm" case.
neu-ulm => Neu-Ulm.

However: "frankfurt-am-main" would result in "Frankfurt-Am-Main".
#23

@dirkschumacher dirkschumacher force-pushed the better_naming branch 2 times, most recently from 6cba519 to de732c5 Compare March 26, 2016 09:47
@johnjohndoe
Copy link
Member

First thing which comes to my mind: we should start writing tests for the Javascript magic.

@dirkschumacher
Copy link
Member Author

Yes! But not as part of this PR :)
Maybe open an issue to set up a testing system.

@torfsen
Copy link
Member

torfsen commented Mar 26, 2016

How about putting the city's display name into the city's meta data? I don't see the point of having a very complex automatism to turn city IDs into display names if we have the meta-data system in place already.

@johnjohndoe
Copy link
Member

This solution was inspired by the thought not to store city names in multiple places (file names, README.md, and now metadata). This might get annoying in terms of maintenance.

@dirkschumacher
Copy link
Member Author

I would still opt for a specific meta-data file where all cities are named. This leads to double entries, but it removes the problem with querying the github api and doing some filename transformation.

We could write an automated test that checks if all .json files are in the cities.json meta-data list.

@torfsen
Copy link
Member

torfsen commented Mar 26, 2016

I, too, like the idea of not repeating the information too often. One possibility would be to generate the ID from the display name instead of the other way round:

  1. Use the full city name with proper capitalization as the filename, but replace spaces with underscores: Frankfurt_am_Main.json.
  2. From these filenames, generate a city's ID by conversion to lowercase and replacement of _ by -: frankfurt-am-main.

The IDs can be generated when we load the city list.

However, I think it's still worth to simply have a separate cities.json file that lists the available cities. While it would make maintenance slightly more cumbersome it saves us a request to the GitHub API which makes the app portable. In addition the only time that you need to touch that information is when you add a new city. I think that's OK. That would be a separate PR, though.

@johnjohndoe
Copy link
Member

The argument to make the app portable by not depending on the API of GitHub convinces me.
+1 for a refactoring.

@johnjohndoe johnjohndoe merged commit d29b28e into wo-ist-markt:master Apr 4, 2016
joergreichert referenced this pull request in joergreichert/wo-ist-testzentrum.github.io Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants