smart-on-fhir / health-cards

Health Cards Framework: implementation guide and supporting material
Other
260 stars 84 forks source link

Change operation name to FHIR convention #80

Closed yunwwang closed 3 years ago

yunwwang commented 3 years ago

FHIR convention uses lower case letter for operation name, such as "$validate-code". http://build.fhir.org/operationslist.html

Suggest change operation $HealthWallet.issueVC to $healthwallet-issuevc

jmandel commented 3 years ago

Does FHIR have explicit conventions for naming OperationDefinitions? I see some jurisdiction specific guidance like https://developer.nhs.uk/apis/fhir-policy/naming.html#FHIR-NAME-05 (which uses a mix of capital and lower case letters, for what it's worth) but not universal guidance.

In the examples you listed, the common pattern is verb-noun (e.g., find-matches, submit-data, collect-data), but this is mostly the case when they apply directly to a FHIR resource e.g., Measure/$submit-data), but your proposal is noun-verb -- it's only superficially aligned with the syntax of the FHIR examples.

I'm not sure it's helpful to break current implementations by renaming this operation, especially if there's no ecosystem-wide convention that we'd be aligning to.

yunwwang commented 3 years ago

I found it here: https://confluence.hl7.org/display/FHIR/Guide+to+Designing+Resources:

be lowerCamelCase for elements, UpperCamelCase for resources, be lowercase for operations

jmandel commented 3 years ago

This guidance applies to core FHIR content; it doesn't say much about operations beyond lower-case (as you point out), and different jurisdictions have set different rules. I see a clear cost associated with changing the name of $HealthWallet.issueVc, and I don't see a benefit. Can you clarify the goals of renaming?

zsura commented 3 years ago

I agree that there is a cost associated to renaming the operation but if we do decide to rename it, do we need consider "HealthCard" instead of "HealthWallet". The wallet naming has been dropped everywhere except the operation name.

yunwwang commented 3 years ago

My goal is keep consistency with FHIR core naming convention. A possible naming would be $issue-healthcard. I assume another operation missing (for future development) is $verify-healthcard (section https://smarthealth.cards/#presenting-health-cards-to-a-verifier). I am sure what the "cost" is.

jmandel commented 3 years ago

My goal is keep consistency with FHIR core naming convention.

Given that Health Cards is not part of FHIR core, the value is unclear here. The cost is breaking existing implementations that folks have been working on; if we're going to do so, now is certainly the time! (The cost will only grow.)

Keep in mind that the operation "Grouping" structure ("HealthWallet") is also used in SMART capability discovery; if we changed the operation name, we'd want to change the capability name too.

I very much wouldn't want to make the "common part" a suffix; it should be a prefix, to make the grouping function clear.

@zsura I do agree with your point about the word "wallet". If we were going to rename this operation, I guess I'd want to call it health-cards-issue-vc. Let me try a straw poll; I'll add a follow-up comment with a precise question.

jmandel commented 3 years ago

Please respond to this comment with a thumbs-up if you'd like to rename $HealthWallet.issueVc to $health-cards-issue.

AlexJonsson commented 3 years ago

healthcard-issuevc

jmandel commented 3 years ago

Keep the current name $HealthWallet.issueVc

p2-apple commented 3 years ago

I do agree that it makes sense to align with what we're adding in capabilities, which is health-cards. I'm also thinking we can drop the "VC" part because we're issuing health cards, which are VCs. What I like with the existing proposal is the implicit namespacing using the period, so something like $health-cards.issue seems preferable to me.

jmandel commented 3 years ago

$health-cards.issue has the most votes but doesn't meet Yunwei's syntax requirement which kicked off this issue in the first place. I'll go forward with $health-cards-issue