Skip to content

Rename identity verification routes#844

Merged
jessieay merged 1 commit intomasterfrom
jy-urls-5
Dec 13, 2016
Merged

Rename identity verification routes#844
jessieay merged 1 commit intomasterfrom
jy-urls-5

Conversation

@jessieay
Copy link
Contributor

@jessieay jessieay commented Dec 9, 2016

Why: IDV does not make sense to an end user. Verify does. cc @esgoodman

Open question: It seems like it makes sense to call some stuff IDV on our side since that is the technical term. Or maybe we move everything to be called "Verification" ?

@pkarman
Copy link
Contributor

pkarman commented Dec 12, 2016

I'm +0 on renaming these routes. If the rationale is allowing the user to easily spell out the URL over the phone, then keep it as short as possible seems ideal.

If we do rename, verify is a verb and we try to use nouns in our URLs.

@jessieay
Copy link
Contributor Author

@pkarman this is the "step" that @esgoodman and I agreed on when discussing. She described them as verbs:

sign_up (sign up)
sign_in (sign in)
manage (edit)
recover
verify (idv)

We already updated sign_up routes to use sign_up namespace consistently. This PR represents another step in that direction. These are all a part of issue 197 from identity-private. Do you think that issue should be de-prioritized?

@pkarman
Copy link
Contributor

pkarman commented Dec 12, 2016

based on the work already done, looks like verbs are in the URLs. 🤷 OK.

Copy link
Contributor

@pkarman pkarman left a comment

Choose a reason for hiding this comment

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

lgtm

@jessieay
Copy link
Contributor Author

@pkarman any thoughts on

It seems like it makes sense to call some stuff IDV on our side since that is the technical term. Or maybe we move everything to be called "Verification" ?

@pkarman
Copy link
Contributor

pkarman commented Dec 12, 2016

I think the pattern has been to name things in the code according to their technical meaning to developers.

@jessieay
Copy link
Contributor Author

I am adding the blocked label to this until I discuss with Liz about what to do next

**Why**: IDV does not make sense to an end user. Verify does.
@jessieay
Copy link
Contributor Author

Caught up with @esgoodman on this. We decided to move forward with the re-name proposed in this PR.

Reasoning:

  • idv is not "plain language" whereas "verify" is
  • using the verb rather than the noun is less Railsy, but easier to read and shorter. We are really using it as a namespace so it seems less important to have it be a noun that it would if it were naming controllers after verbs (similar to sign_up being a namespace)

@jessieay jessieay merged commit 1f1e255 into master Dec 13, 2016
@jessieay jessieay deleted the jy-urls-5 branch December 13, 2016 00:47
amoose pushed a commit that referenced this pull request Feb 24, 2017
**Why**: IDV does not make sense to an end user. Verify does.
amoose pushed a commit that referenced this pull request Feb 28, 2017
**Why**: IDV does not make sense to an end user. Verify does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants