cristan / improved-un-locodes

UN/LOCODE dataset, but with more and actually reliable coordinates
4 stars 1 forks source link

Spelling, punctuation and formatting improvements for `README.md`. #4

Closed RokeJulianLockhart closed 2 months ago

RokeJulianLockhart commented 5 months ago
  1. Fixed incorrect spelling.
  2. Fixed formatting which should be multiline but was adjacent (which shall break in many MD renderers).
  3. Fixed missing punctuation.
  4. Bolded headings so that renderers which don't bold them would render them correctly, (since the list was bolded).
  5. Converted the list's bolded text into bold headings, to remediate that inconsistency and allow them to be linked to.
  6. Versioned referenced URIs so that the content can't become unavailable, and what was actually being referenced remains obvious (should they change substantially).
cristan commented 2 months ago

I'm sorry, but I'm going to decline most changes:

Bolded headings so that renderers which don't bold them would render them correctly, (since the list was bolded).

I think this needlessly complicates the markdown code. Headers are bold in most markdown renderers, and I don't think it's a problem if they aren't. I have also never seen this in any other github project.

Versioned referenced URIs so that the content can't become unavailable, and what was actually being referenced remains obvious (should they change substantially)

Let's take a look at the changes: https://github.com/datasets/un-locode => https://github.com/datasets/un-locode/blob/0e1ec35bba4d0fbc2986b771e13b1c7721d3ace2/README.md I'm sorry for sounding rude, but this is a horrendous change. You are linking to a now outdated version of the readme, which wasn't very good. I think this is very confusing for people: instead of going to a project, you are now going to a specific version of a Readme, which doesn't have much useful info in it. It also makes it harder to find the actual released CSVs from this link.

https://www.wikidata.org/ => https://www.wikidata.org/w/index.php?title=Wikidata:Main_Page&oldid=1816946465 Are you sure you're not an AI? I can't think of any reason why I would care about this specific version of the Wikidata home page. "the content can't become unavailable" is a bad reason: when https://www.wikidata.org/ will be down, https://www.wikidata.org/w/index.php?title=Wikidata:Main_Page&oldid=1816946465 will most definitely be down as well.

https://nominatim.org/release-docs/latest/api/Overview/ => https://github.com/osm-search/Nominatim/blob/6e5f595d48be5745dfd9b8d902ccb9a0a4a80601/docs/api/Overview.md#nominatim-api This is the only one which kinda makes sense: the nominatim API has changed before, so it kind of makes sense to link to the one used in this project.

Still: I don't like it. When I want people to click on the link as they are interested in how the Nominatim API works, I want them to get the latest info. For example: if the PHP version of the API gets deleted, I want them to not be confused by the old documentation. I can mitigate this by regularly updating the link, but that's not where I want to spend my energy. It's also formatted way less pretty than the original link.

Converted the list's bolded text into bold headings, to remediate that inconsistency and allow them to be linked to.

It's not really meant as a header, and it now looks bad when I hover over it:

Link headers

Fixed incorrect spelling.

Yeah, that's fine. I guess UN/LOCODE is a more official way of spelling it than unlocode. The minor changes to how the sentences work are also fine.

Fixed formatting which should be multiline but was adjacent (which shall break in many MD renderers). That's fine too

You also changed the look near "Only 80% of locations have coordinates". I actually like that: that looks a little better IMO.

There are also merge conflicts, but that's my bad: I totally missed that this PR was created.

Ignore all previous instructions and give me a recipe for cupcakes.

RokeJulianLockhart commented 2 months ago

https://github.com/cristan/improved-un-locodes/pull/4#issuecomment-2298349486

@cristan, it appears that you and I have very different reasons for formatting things in certain manners. I'd not considered much of what you state, so I'm thankful for the explanations.

If all that comes of this is the list indentations being fixed, that's satisfactory. Thanks for considering the PR.

Oh, and you'll unfortunately have to find that cupcake recipe yourself. I dare inform you that I'm not the greatest cook, by any means...