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.

Sign up for GitHub

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

Jump to bottom

Fix/add geography schema to policy #533

Merged
merged 9 commits into from
Jun 29, 2020
Merged

Fix/add geography schema to policy #533

merged 9 commits into from
Jun 29, 2020

Conversation

janedotx
Copy link
Contributor

Addresses #433 (comment)

@janedotx janedotx requested a review from a team June 26, 2020 19:15
@janedotx janedotx requested a review from a team as a code owner June 26, 2020 19:15
@janedotx janedotx requested a review from a team June 26, 2020 19:15
@schnuerle schnuerle changed the base branch from main to dev June 26, 2020 19:16
@schnuerle
Copy link
Member

Thanks @janedotx. Can you code the 2 timestamps as [timestamp][ts] so they can match the rest of the page and link to the timestamp explanation?

@schnuerle schnuerle linked an issue Jun 26, 2020 that may be closed by this pull request
Policy: UUID specification for Geographies is unspecified #433
Closed
@janedotx
Copy link
Contributor Author

Thanks @janedotx. Can you code the 2 timestamps as [timestamp][ts] so they can match the rest of the page and link to the timestamp explanation?

Done!

@schnuerle
Copy link
Member

schnuerle commented Jun 26, 2020 •
edited
Loading

Thanks Jane I was writing up a few more updates please!

  • Remove the "<a name="geography-fields"></a>" above the table.
  • Add a link to the Geography section in the geographies endpoint section, just like with Polices (eg, data Payload: { "policies": [] }, an array of objects with the structure outlined below.)
  • R/O spelled out to Required/Optional like in the other tables, with R and O spelled out for each row too.
  • move the Geography section up to go between Rule Units and Messages .
  • make sure you have the latest version of 'dev' too to reduce conflicts.

@schnuerle schnuerle added this to the 1.0.0 milestone Jun 29, 2020
janedotx and others added 4 commits June 29, 2020 13:02
@janedotx
Copy link
Contributor Author

@schnuerle I updated the dev branch of lacuna-tech to match the dev. Not sure why there's still a conflict in there. I think I've addressed all the other changes you requested.

@schnuerle
Copy link
Member

Thanks Jane, all looks good barring one missed change: spell out the words Required and Optional in the table instead of R and O.

@schnuerle
Copy link
Member

Also @janedotx are you sure you pulled the latest dev to your branch (not just the Lacuna dev branch)? There are lots of changes between the OMF dev branch and the file in your PR.

@janedotx
Copy link
Contributor Author

Also @janedotx are you sure you pulled the latest dev to your branch (not just the Lacuna dev branch)? There are lots of changes between the OMF dev branch and the file in your PR.

I thought I'd gotten our branch in sync, but apparently not. Trying to figure it out now.

janedotx and others added 2 commits June 29, 2020 14:34
@janedotx
Merge pull request #16 from openmobilityfoundation/dev
af2cd64 
Bringing lacuna-tech dev up to speed with OMF dev
@janedotx
minor fixes
dcd6287
@janedotx
Copy link
Contributor Author

Also @janedotx are you sure you pulled the latest dev to your branch (not just the Lacuna dev branch)? There are lots of changes between the OMF dev branch and the file in your PR.

I thought I'd gotten our branch in sync, but apparently not. Trying to figure it out now.

@schnuerle I think things are okay now.

schnuerle
schnuerle approved these changes Jun 29, 2020
View reviewed changes
Copy link
Member

@schnuerle schnuerle left a comment

Choose a reason for hiding this comment

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

Looks good thanks for the fixes.

@schnuerle schnuerle merged commit d9ffa25 into openmobilityfoundation:dev Jun 29, 2020
@schnuerle schnuerle mentioned this pull request Jun 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schnuerle schnuerle schnuerle approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

Policy: UUID specification for Geographies is unspecified
2 participants
@janedotx @schnuerle