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

feat: remove the use of mix-type fields for transportation access restrictions #85

Conversation

bastiaanvanassche-tomtom
Copy link
Contributor

Based on:

this PR tackles the use of mix-type fields for transportation access restrictions

sanjibdutta1
sanjibdutta1 previously approved these changes Nov 27, 2023
rickyyuan07
rickyyuan07 previously approved these changes Nov 28, 2023
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.

I'd like to discuss alternative names for accessType.

@bastiaanvanassche-tomtom bastiaanvanassche-tomtom dismissed stale reviews from brad-richardson, rickyyuan07, and sanjibdutta1 February 10, 2024 01:45

The merge-base changed after approval.

@vcschapp
Copy link
Collaborator

@bastiaanvanassche-tomtom Is it fair to say this PR captured what we wanted to do, but we never managed to merge it?

@vcschapp
Copy link
Collaborator

I'd like to discuss alternative names for accessType.

Can we change accessType to just access? It feels more right to me.

@vcschapp
Copy link
Collaborator

Plan agreed in the 2024-02-20 transportation TF main business meeting:

  1. Bastiaan will resuscitate this PR, clear merge conflicts, get any needed approvals, and merge it.
  2. After this, he will follow with two smaller PRs to make two adjustments:
    1. Rename accessType along the lines mentioned in this comment.
    2. Make the one-liner change to the combobulator to bring it into sync with the schema.

@bastiaanvanassche-tomtom bastiaanvanassche-tomtom force-pushed the users/bastiaanvanassche-tomtom/transportation_remove_mix_type_access_field branch 4 times, most recently from e9c821a to ef4cd28 Compare February 29, 2024 16:16
@bastiaanvanassche-tomtom bastiaanvanassche-tomtom force-pushed the users/bastiaanvanassche-tomtom/transportation_remove_mix_type_access_field branch from ef4cd28 to ee1d56c Compare February 29, 2024 16:19
@vcschapp
Copy link
Collaborator

Plan agreed in the 2024-02-20 transportation TF main business meeting:

  1. Bastiaan will resuscitate this PR, clear merge conflicts, get any needed approvals, and merge it.

  2. After this, he will follow with two smaller PRs to make two adjustments:

    1. Rename accessType along the lines mentioned in this comment.
    2. Make the one-liner change to the combobulator to bring it into sync with the schema.

These TODOs are now tracked in https://github.com/OvertureMaps/tf-transportation/issues/101. So we really want to push to get this one reviewed and merged as-is.

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.

        ._a
        |8P
      __| __
    .-------,       w
     \  .  /     w
      )a:f(           .
     /_    \          |
    (___2___)      `. _ ,' 
      .c@a.  _______ ( )._______
 ~   `Y888P       ~  ~-
      ~~~~ ~  ~~     =-

@vcschapp vcschapp merged commit 43fcdfe into dev Mar 4, 2024
2 checks passed
@vcschapp vcschapp deleted the users/bastiaanvanassche-tomtom/transportation_remove_mix_type_access_field branch March 4, 2024 19:13
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