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 country_code to location #108

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Add country_code to location #108

merged 1 commit into from
Apr 14, 2024

Conversation

dbrgn
Copy link
Contributor

@dbrgn dbrgn commented Apr 11, 2024

Add an ISO-3166 alpha-2 country code to the location object.

Note: One could argue that - since the location becomes optional with #106 - the country code can be used for umbrella organizations and spaces without fixed location as well and should be outside of the location key. On the other hand, the same thing county for the time zone, so I decided to stick with location.

Resolves #74.

@dbrgn dbrgn added this to the API v15 milestone Apr 11, 2024
@dbrgn dbrgn requested a review from a team April 11, 2024 21:27
Copy link
Member

@rnestler rnestler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: One could argue that - since the location becomes optional with #106 - the country code can be used for umbrella organizations and spaces without fixed location as well and should be outside of the location key.

Can we have location with just the country code set? Then this point would be solved.

Another thing: Should we validate the country code? We could sanity check lon/lat with it for example.

@dbrgn
Copy link
Contributor Author

dbrgn commented Apr 14, 2024

Can we have location with just the country code set? Then this point would be solved.

We could think about that. In #106, we made the entire location field optional. Instead of, or in addition to that, we could make lat/lon optional. This would mean that you could have a location with an address set, but without coordinates.

But let's discuss that in a dedicated issue. (Edit: #112)

Another thing: Should we validate the country code? We could sanity check lon/lat with it for example.

Yeah, wouldn't hurt to add that to the validator UI: spaceapi-community/validator-web#3

(Note: The lat/lon check you're probably thinking of is only part of the web UI, not the validator API.)

@dbrgn dbrgn merged commit cf6a350 into master Apr 14, 2024
8 checks passed
@dbrgn dbrgn deleted the country-code branch April 14, 2024 20:38
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.

Country code
3 participants