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

Rename publish_date to published_date for a geography object #597

Merged
merged 2 commits into from
Nov 11, 2020

Conversation

fractalf
Copy link
Contributor

A policy object contains a property published_date which is the exact
same thing as a geography object's publish_date. These should therefore
also have the same name to prevent errors and confusion.


name: Default
about: Suggest changes to MDS
title: Rename publish_date to published_date for a geography object


MDS Pull Request

Thank you for your contribution! Please review our OMF contributing page to understand guidelines and policies for participation, and our Code of Conduct page.

To avoid complications and help make the Review process as smooth as possible, make sure to:

  1. Target dev branch. Please ensure you are targeting dev, not main.
  2. Keep the "Allow edits from maintainers" button checked to help us resolve some issues for you.
  3. Be ready to resolve any merge conflicts before we approve your Pull Request.
  4. Have an up to date profile, per our Github community profile guildance.

Explain pull request

A policy object contains a property published_date which is the exact
same thing as a geography object's publish_date. These should therefore
also have the same name to prevent errors and confusion.

Is this a breaking change

A breaking change would require consumers or implementors of the API to modify their code for it to continue to function (ex: renaming of a required field or the change in data type of an existing field). A non-breaking change would allow existing code to continue to function (ex: addition of an optional field or the creation of a new optional endpoint).

  • Yes, breaking

Impacted Spec

Which spec(s) will this pull request impact?

  • policy

Additional context

A policy object contains a property published_date which is the exact
same thing as a geography object's publish_date. These should therefore
also have the same name to prevent errors and confusion.
@fractalf fractalf requested a review from a team November 10, 2020 08:51
@fractalf fractalf requested a review from a team as a code owner November 10, 2020 08:51
@CLAassistant
Copy link

CLAassistant commented Nov 10, 2020

CLA assistant check
All committers have signed the CLA.

@schnuerle schnuerle added Geography Items related to the Geography API Policy Specific to the Policy API labels Nov 10, 2020
@schnuerle schnuerle added this to the 2.0.0 milestone Nov 10, 2020
@schnuerle
Copy link
Member

Added some tags and slated for 2.0.0 release since changing the field names. However since this looks like only Geography is being changed, and that hasn't released yet, maybe it can be in 1.1.0.

Also @fractalf make sure to sign the CLA.

policy/README.md Outdated Show resolved Hide resolved
@fractalf
Copy link
Contributor Author

fractalf commented Nov 11, 2020

@schnuerle
CLA signed

I have limited knownledge to the structure/branches of this project, I just followed the instructions and edited the dev branch.
If the dev branch is targetet for 2.0.0 then I understand.

If you feel this change could be in 1.1.0, I'm happy to make another PR, but the I need to know where to put it? I saw no branch targeting 1.1.0

That way this PR is not breaking and be included in 1.1.0. Can be changed in next major release.
@schnuerle
Copy link
Member

Thanks for signing.

There is no branch for releases until the OMF makes a release candidate. Everything pretty much gets pointed to 'dev' like you have done already, before going to OMF tech council and board for release approval. The Milestone tags help categorize features and PRs for releases until they are ready. So you are doing things right!

This whole PR can go into 1.1.0, except for the geography field name change within policy, per our conversation here, since that would be breaking. So I went in and edited that away for now.

@schnuerle schnuerle modified the milestones: 2.0.0, 1.1.0 Nov 11, 2020
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.

Good for 1.1.0 changes to the new geography endpoint.

@schnuerle schnuerle merged commit 40a4b3a into openmobilityfoundation:dev Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Geography Items related to the Geography API Policy Specific to the Policy API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants