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

refactor: rename PinStatus id → requestid #63

Merged
merged 4 commits into from
Sep 18, 2020
Merged

refactor: rename PinStatus id → requestid #63

merged 4 commits into from
Sep 18, 2020

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 15, 2020

This is an attempt to clarify the difference between cid identifier in request and id in response:

  • adds "Identifiers" section to the docs and clarifies purpose of each
  • BREAKING CHANGE: id is renamed to requestid
    • I believe this will make it easier for people to reason requestid vs cid, but can restore old field name if community decides it is not worth the hassle.

PREVIEW: https://ipfs.github.io/pinning-services-api-spec/#specUrl=https://raw.githubusercontent.com/ipfs/pinning-services-api-spec/docs/pid/ipfs-pinning-service.yaml

cc @obo20 @andrew @aschmahmann @GregTheGreek @priom @jsign @sanderpick @andrewxhill @ipfs/wg-pinning-services

Please provide feedback (even if its just 👍 / 👎)

This is an attempt to clarify the difference between idenfitier in
request and one in response, adds docs and clarifies names.

BREAKING CHANGE: id is renamed to pid
@hsanjuan
Copy link

I would have thought pid corresponds to peerID (which incidentally looks like a CID), not PinID. For this reason it does not seem to me like the best name. Perhaps it does not hurt to be specific and name this requestid or trackid.

@jacobheun
Copy link
Collaborator

If we're going to rename it, +1 to requestid, just remove the ambiguity. I saw pid and immediately thought process id, so it's clear to me it's not really alleviating confusion.

@andrew
Copy link

andrew commented Sep 15, 2020

I saw pid and immediately thought process id

+1 on that, also +1 on @hsanjuan's requestid

@jessicaschilling
Copy link
Contributor

+1 to @jacobheun and @hsanjuan, plus "pid" has some unpleasant real-world meaning.

As suggested in comments starting at
#63 (comment)
@lidel lidel changed the title refactor: rename PinStatus id → pid refactor: rename PinStatus id → requestid Sep 15, 2020
@lidel
Copy link
Member Author

lidel commented Sep 15, 2020

Thanks for quick feedback! renamed it to requestid (which indeed removes any surface for ambiguity)
and updated PR to reflect the change

ipfs-pinning-service.yaml Outdated Show resolved Hide resolved
@obo20
Copy link

obo20 commented Sep 17, 2020

Any thoughts on objectid, serviceid, or psid(pinning service id)?

requestid works for me, but it makes me immediately think of the api request itself which could be confusing to some people seeing as you may make multiple api requests to a single ID over its life cycle.

@lidel
Copy link
Member Author

lidel commented Sep 17, 2020

I think those are also fine, but requestid is tiny bit better as it enables us to explain things in simpler words:

  • Every (HTTP) request to create a pin object at a pinning service has a unique ID
  • You use this ID to check the status of ongoing pin request, and for revoking / replacing it later.

@lidel lidel merged commit c68b267 into master Sep 18, 2020
@lidel lidel deleted the docs/pid branch September 18, 2020 11:37
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.

6 participants