-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Minor] Add is_land and is_territorial to divisions #303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stepps00
changed the title
[WIP] Add is_land and is_territorial to divisions
Add is_land and is_territorial to divisions
Dec 5, 2024
jonahadkins
previously approved these changes
Dec 5, 2024
DavidKarlas
previously approved these changes
Dec 9, 2024
cc @vcschapp / @TristanDiet-TomTom for sign off on the series of divisions schema changes. |
TristanDiet-TomTom
previously approved these changes
Dec 11, 2024
vcschapp
reviewed
Dec 11, 2024
vcschapp
added
the
change type - minor 🤏
Minor schema change. See https://lf-overturemaps.atlassian.net/wiki/x/GgDa
label
Dec 13, 2024
vcschapp
changed the title
Add is_land and is_territorial to divisions
[Minor] Add is_land and is_territorial to divisions
Dec 13, 2024
DavidKarlas
dismissed stale reviews from TristanDiet-TomTom, jonahadkins, and themself
via
December 18, 2024 12:21
bb753bd
DavidKarlas
approved these changes
Dec 18, 2024
TristanDiet-TomTom
approved these changes
Dec 18, 2024
jonahadkins
approved these changes
Jan 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_good ✅
vcschapp
reviewed
Jan 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ship it!
vcschapp
approved these changes
Jan 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
change type - minor 🤏
Minor schema change. See https://lf-overturemaps.atlassian.net/wiki/x/GgDa
schema
theme - divisions 🏴
Divisions theme
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Category
What kind of change is this?
Please select one of the following four options.
Consult Pull request merging criteria for a description of each category.
Description
This fixes a portion of https://github.com/OvertureMaps/tf-admin/issues/82 and replaces #294.
This pull request adds a
is_land
andis_territorial
properties to division_area and division_boundary. This replaces a proposal to addis_processing
andis_rendering
, using less case-specific language in property names.is_land
: For the divisions theme, this field would indicate whether or not the feature geometry represents the non-maritime "land" boundary, which can be used for map rendering, cartographic display, and similar purposes.is_territorial
: For the divisions theme, this field would indicate whether or not the feature geometry represents the full territorial boundary or claim of a feature.These new properties will eventually replace the existing
class
property, butclass
has not been removed (yet).Examples and counter examples have been added. I've also attempted to add a contraint (
anyOf
) to the newis_territorial
andis_land
properties, requiring that one or both of those booleans must be true.Reference
List of relevant links to GitHub issues, PRs, and other documentation.
Testing
Brief description of the testing done for this change showing why you are confident it works as expected and does not introduce regressions. Provide sample output data where appropriate.
TODO.
Checklist
Checklist of tasks commonly-associated with schema pull requests. Please review the relevant checklists and ensure you do all the tasks that are required for the change you made.
A
but is not intended to test propertyA
's validity, and you made a schema change that invalidates propertyA
in that counterexample, fix the counterexample to align it with your schema change.Documentation Website
Update the hyperlink below to put the pull request number in.
Docs preview for this PR.