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

Remove "value" as required element of address_level #283

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

jwass
Copy link
Collaborator

@jwass jwass commented Sep 30, 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 bug in the schema where unknown address_levels were disallowed. Allow them by still requiring the
addressLevelContainer struct, but make the "value" field optional.

Reference

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

  1. TODO.

Testing

jv

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.

@jwass jwass changed the title Allow address_levels to be null Remove "value" as required element of address_level Sep 30, 2024
DavidKarlas
DavidKarlas previously approved these changes Oct 2, 2024
@vcschapp
Copy link
Collaborator

vcschapp commented Oct 2, 2024

Note to myself: We had meant to allow null but never did. Now using {} placeholder instead of allowing null.

@vcschapp
Copy link
Collaborator

vcschapp commented Oct 2, 2024

Note to myself: We had meant to allow null but never did. Now using {} placeholder instead of allowing null.

Reason for {} is that arrays with explicit null value are somewhat uncommon/surprising. We feel that an existing object/structure with missing fields is a more common/less surprising data structure.

Agreed 2024-10-02 Schema TF meeting: We'll follow this approach and try to avoid explicit null value in array.

There's an open point about whether address_levels should be empty if it has no meaningful content. Punting on that.

@vcschapp vcschapp merged commit e376796 into dev Oct 2, 2024
2 checks passed
@vcschapp vcschapp deleted the allow_null_address_levels branch October 15, 2024 19:11
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.

5 participants