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

Deprecated ID proposal #38

Closed
wants to merge 4 commits into from
Closed

Conversation

Floppy
Copy link

@Floppy Floppy commented Oct 9, 2014

As reported in #37, old IDs should be marked as deprecated, not removed. This is my proposal for how this could be represented, using the old cc-zero ID as an example.

@Floppy Floppy mentioned this pull request Oct 9, 2014
@Floppy
Copy link
Author

Floppy commented Oct 9, 2014

If the idea seems sound, I'll reopen the PR on a fresh clone so it doesn't include those merge commits - sorry about that.

@mlinksva
Copy link
Contributor

mlinksva commented Oct 9, 2014

Seems to have more than necessary for mere id changes. Why not just
{
"id": "cc-zero",
"same-as": "CC0-1.0"
}

@Floppy
Copy link
Author

Floppy commented Oct 9, 2014

That would also work, but the URL for the new one is there so that clients can traverse in a HATEOAS kind of way, rather than expecting a client to know how to construct the relevant URL from an ID.

@mlinksva
Copy link
Contributor

mlinksva commented Oct 9, 2014

Ah, I see. Would what you proposed un-break your client?

@Floppy
Copy link
Author

Floppy commented Oct 9, 2014

If we updated our client to handle it, yes - we could follow the URLs. TBH, we construct the URLs ourselves anyway, so your proposal would also work, but you know, WWRFD? :)

@mlinksva
Copy link
Contributor

mlinksva commented Oct 9, 2014

Is that an easier update for your client than just updating it to use the new URLs?

I'm sorry for breaking it (I did not know there were any actual consumers, though I asked if there were).

The idea is the new IDs won't ever need to be changed. But if they were it would seem to me we should just not break clients at all, rather than give them a slightly easier update.

@Floppy
Copy link
Author

Floppy commented Oct 9, 2014

It's more we have the old IDs in our database that stops it working, though we do have mapping to the new ones. We don't need a fix now, we've handled it, but this is more to propose a mechanism for deprecation in the future. Change always happens ;)

@mlinksva
Copy link
Contributor

Is there a best practice for JSON APIs and updating expected field values?

@rufuspollock
Copy link
Member

@mlinksva @Floppy what remains here? If this is something that would fix brokenness if would be nice to get in

@mlinksva
Copy link
Contributor

I don't think there's anything that remains with this PR, I'm closing without merging.

We might want to have a previously-known-as-id column in future csv and reflect that in generated json so that a client that wants to future proof against id changes can do so.

@mlinksva mlinksva closed this Apr 19, 2015
This pull request was closed.
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.

4 participants