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

Proposed change of hgv_time_restrictions field to Time Domain from GDF 5.1 #2063

Merged
merged 4 commits into from Mar 18, 2022

Conversation

roman-ianivskyy
Copy link
Contributor

@roman-ianivskyy roman-ianivskyy commented Mar 14, 2022

… and shorter format of Time Domain from GDF 5.1

  • Update tests
  • Update docs

@roman-ianivskyy roman-ianivskyy changed the title Proposed change of hgv_time_restrictions field to a more standardized… Proposed change of hgv_time_restrictions field to Time Domain from GDF 5.1 Mar 14, 2022
@roman-ianivskyy
Copy link
Contributor Author

Hello, @nvkelso, could you please check this proposed change to time format for HGV restrictions? I assume it was not yet included in Tilezen datasources. Or if it was already released - what is the process for such breaking changes?

docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
docs/layers.md Outdated Show resolved Hide resolved
Copy link
Member

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

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

Please address the nits in the documentation and then I'll approve.

Tilezen uses documents our implementation of SemVer in

Since this is an optional roads layer property, it isn't explicitly covered in the contract (which is mostly focused on layers, kinds, and min_zoom changes. Adding optional properties is in minor version. Modifying the value definitions (since this is really a large reclassification) would fall into MINOR or PATCH territory. But we'll tag it in the upcoming v1.9 release and can merge this PR when you're ready with the changes.

We've never added support for it in the mainline vector-datasource repo here, which also gives some more flexibility.

Copy link
Member

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

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

Left one nit about number of weeks in a year.

@nvkelso nvkelso merged commit 6e05719 into tilezen:master Mar 18, 2022
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.

None yet

2 participants