codeforgermany / click_that_hood

A game where users must identify a city's neighborhoods as fast as possible
http://click-that-hood.com
MIT License
450 stars 639 forks source link

organizing the code #308

Closed Gsirawan closed 3 years ago

Gsirawan commented 4 years ago

No modified has applied on the code. Just improved to readable code.

specious commented 3 years ago

@Gsirawan, it looks like you ran this code through a formatting tool.

I'm looking a the data files in this project, and even through most of them are minified for compactness at this time, it probably makes sense to have them formatted, because then we'd be able to compare and discuss diffs to the data.

I'm choosing to merge this contribution.

fnogatz commented 3 years ago

Hey @specious, thank you for the effort you put into the project the recent days! We could discuss the question format JSON or not? in a separate issue. My first impression is to have a minified version as part of a build step for smaller file size, but to require the *.metadata.json files to be formatted, e.g., via Prettier. In contrast, I don't think we will ever discuss diffs of GeoJSON, so they could always be minified.

specious commented 3 years ago

I agree we should open a separate issue on this.

specious commented 2 years ago

@fnogatz, I think geojson is meant to be human readable. Sooner or later we'll likely be discussing changes to the shapes, as in: #207

I've converted all geojson data to a formatted appearance with prettier: fb1c363b3624a256d42f00788fca96d9faf43a45

epaulson commented 2 years ago

I really don't think GeoJSON is meant to be human-editable, at least not at the scale of the shapes that we're using. I don't think we're going to be looking at textual diffs for shapes - if people send us new shapes in PRs, I'm just going to open both files in QGIS or something and look at the old and new as layers in a GUI, and not look at the textual difference. (There's no guarantee that an GIS program would serialize a GeoJSON in the same order anyway, so even a prettier'd diff may not be useful)

I don't have strong feelings about formatting them or not, I just don't think it's likely to be necessary for discussions.

specious commented 2 years ago

You'll be able to see how shapes were altered between revisions by looking at a diff.

specious commented 2 years ago

As the readme now says, after saving the data file, just normalize it with:

npx prettier {file}.geojson
epaulson commented 2 years ago

Those diffs are going to be great big lists of lat/lons. I am not smart enough to be able to visualize those changes in my head🙂

I don't think this matters in practice, so as long as we're willing to fix up PRs on behalf of submitters that might have all the right data but are just formatted incorrectly I don't think it's a big deal. But the nice thing about this game is it's pretty easy for people to get started with - the idea was that you could literally download data from your city's open data portal and in a GUI get pretty much everything set up correctly. Let's not add too many more things for people to do in order to contribute.

specious commented 2 years ago

They don't have to do anything extra. People can open a PR and maintainers can normalize the data before committing.

Having the data stored in a uniform format will prove to add value over time.

specious commented 2 years ago

Also, if the data isn't formatted, that currently doesn't prevent deployment. You'll just see an error from the automatically triggered linter task.

specious commented 2 years ago

I know this might sound weird, but I'm probably not the only person out there who sometimes edits geojson files in vim. Also, I sometimes paste it into geojson.io and edit the code to change the shapes.