Skip to content

Conversation

@stepps00
Copy link
Contributor

@stepps00 stepps00 commented Dec 2, 2024

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.

  1. Cosmetic change.
  2. Documentation change by member.
  3. Documentation change by Overture tech writer.
  4. Material change.

Description

This fixes a portion of https://github.com/OvertureMaps/tf-admin/issues/82 and replaces #294.

This pull request adds a class property to the divisons schema.

class: For the divisions type, this field would explicitly call out the type of locality (hamlet, city, etc.) being represented. As a v1, we'd essentially repurpose the local_type en value to create this enum.

As a follow up, a prominence property and usage (is_processing / is_rendering?) will likely be added to the divisions schema.

Reference

List of relevant links to GitHub issues, PRs, and other documentation.

https://github.com/OvertureMaps/tf-admin/issues/82
#294

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.

  1. Add relevant examples.
  2. Add relevant counterexamples.
  3. Update any counterexamples that became obsolete. For example, if a counterexample uses property A but is not intended to test property A's validity, and you made a schema change that invalidates property A in that counterexample, fix the counterexample to align it with your schema change.
  4. Update in-schema documentation using plain English written in complete sentences, if an update is required.
  5. Update Docusaurus documentation, if an update is required.
  6. Review change with Overture technical writer to ensure any advanced documentation needs will be taken care of, unless the change is trivial and would not affect the documentation.

Documentation Website

Update the hyperlink below to put the pull request number in.

Docs preview for this PR.

@stepps00 stepps00 self-assigned this Dec 2, 2024
@vcschapp vcschapp requested review from DavidKarlas, TristanDiet-TomTom, jwass and vcschapp and removed request for vcschapp December 4, 2024 15:40
Copy link
Collaborator

@vcschapp vcschapp left a comment

Choose a reason for hiding this comment

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

Couple checklist items outstanding:

  1. Can you add examples and counterexamples please?
  2. Chat with Dana about any Docusaurus docs updates for divisions page?

@stepps00 stepps00 changed the title [WIP] Add class to divisions schema Add class to divisions schema Dec 4, 2024
jonahadkins
jonahadkins previously approved these changes Dec 5, 2024
Copy link
Contributor

@jonahadkins jonahadkins left a comment

Choose a reason for hiding this comment

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

🆗

sasastanojkov
sasastanojkov previously approved these changes Dec 6, 2024
DavidKarlas
DavidKarlas previously approved these changes Dec 9, 2024
@stepps00
Copy link
Contributor Author

stepps00 commented Dec 9, 2024

cc @vcschapp / @TristanDiet-TomTom for sign off on the series of divisions schema changes.

@vcschapp
Copy link
Collaborator

Changes are approved by all needed approvers, but I pushed a cosmetic commit to fix the "missing newline at end of file" issue in examples/divisions/division/class.yaml and counterexamples/divisions/division/bad-class.yml. Literally nothing changed except the addition of a newline at the end of those files.

I'll merge based on the approvals already given.

@vcschapp vcschapp merged commit 137dfcd into dev Dec 11, 2024
2 checks passed
@vcschapp vcschapp deleted the stepps00/class-divisions branch December 11, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants