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

add 266 USA airports #141

Merged
merged 2 commits into from
Aug 27, 2024
Merged

add 266 USA airports #141

merged 2 commits into from
Aug 27, 2024

Conversation

maidough
Copy link

No description provided.

@eatdostacos
Copy link
Contributor

I noticed that the latitude and longitude for the new airports are strings which doesn't follow the convention in the rest of the file. Thanks for all the additions!

@maidough
Copy link
Author

oops, that was a mistake. fixed

@mwgg mwgg merged commit 12f0dba into mwgg:master Aug 27, 2024
1 check passed
@mwgg
Copy link
Owner

mwgg commented Aug 27, 2024

Thanks, missed that too!

@mwgg
Copy link
Owner

mwgg commented Aug 27, 2024

Added that to the github actions, it will now automatically check it on every PR.

@alexander-san
Copy link
Contributor

Just want to chime in here @mwgg :

  • Can the key for an airport be anything? Because I was under the impression that until now we ensured the airport key is of length = 4. In the case of non-ICAO airports (e.g. X05 - great airport btw., we pre-fixed this with 'K' to indicate the general location of the airport (e.g. USA)
  • I am also good with changing away from the airport key having to be 4, however, the airports can most certainly not have a 3 digit identifier. This is wrong and I am sure that there are applications out there where parsing this could result in problems.

IMO, while not correct, I like the idea of prefixing it with a 'K'. However, we must certainly address this from a consistency perspective because right now we do both...

@mwgg
Copy link
Owner

mwgg commented Aug 28, 2024

The key should match whatever is in the icao field. Many years ago, when it was a MySQL table in a project of mine, that's how it was exported for this repo, and that's what it says in the README. Obviously it's not always ICAO, sometimes it's the FAA identifier.

I do see that we have FAA identifiers both ways. About 200 entries with the 3-letter identifier and way more with the K prefix. For the sake of consistency I agree that we should have it the same way, and the K prefix is a common way of denoting FAA identifiers.

I've changed the existing 3-letter entries, and added a check to be performed on each PR to ensure it's always 4, as well as verifying the key matches the icao field.

#143

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.

4 participants