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

Deprecate use of the string name field in favour of AppMetadata #506

Closed
kriswest opened this issue Nov 17, 2021 · 4 comments · Fixed by #722
Closed

Deprecate use of the string name field in favour of AppMetadata #506

kriswest opened this issue Nov 17, 2021 · 4 comments · Fixed by #722
Labels
api FDC3 API Working Group channels feeds & transactions Channels, Feeds & Transactions Discussion Group enhancement New feature or request

Comments

@kriswest
Copy link
Contributor

Enhancement Request

In FDC3 1.2 we introduced the TargetApp type:

type TargetApp = string | AppMetadata;

to help us transition from using the string name field in several places (intent resolutions, targeted raisedIntents etc.) to using the AppMetadata type. Its also been suggested that we should be encouraging users of the API to use the full AppMetadata Object returned by another call (e.g. findIntent, raiseIntent's IntentResolution).

Should we:

  • deprecate the use of the name field alone
  • formally encourage Desktop Agent implementations to return AppMetadata rather than name (e.g. in IntentResolution's source field via the application if the SHOULD keyword in the spec)
  • provide clear advice and examples of using the AppMetadata objects in the spec and documentation.

n.b. name was not deprecated when TargetApp/AppMetadata use was introduced - that should happen before its removed, despite the fact that our next version is a major release.

@kriswest kriswest added enhancement New feature or request api FDC3 API Working Group labels Nov 17, 2021
@kriswest
Copy link
Contributor Author

@pbaize I've raised this issue in response to your recent comments on this topic + my own feelings on the inelegance of the name field (when AppD supports multiple apps with the same name). Hence, your input would be appreciated.

@pbaize
Copy link

pbaize commented Nov 30, 2021

I'm in full agreement here.

I'd even go as far as asking whether we should release one more 1.x version for deprecations.

@kriswest
Copy link
Contributor Author

As per comment in discussion #678:
#678 (reply in thread)

The proposed changes are:

  • AppMetadata's name field goes from required to optional and appId from optional to required:
interface AppMetadata {
  name: string;
  appId?: string;
  version?: string;
  title?: string;
  ... etc.
}

changes to:

interface AppMetadata {
  appId: string;
  name?: string;
  version?: string;
  title?: string;
  ... etc.
}
  • We would then drop the type TargetApp type, which is just string | AppMetadata and helps us avoid duplicate function signatures currently, and split the function signatures up, with the versions using name becoming deprecated. I.e.:
interface DesktopAgent {
  // apps
  open(app: TargetApp, context?: Context): Promise<void>;
  ...
  // intents
  ...
  raiseIntent(intent: string, context: Context, app?: TargetApp): Promise<IntentResolution>;
  raiseIntentForContext(context: Context, app?: TargetApp): Promise<IntentResolution>;
  ...
}

changes to:

interface DesktopAgent {
  // apps
  open(app: AppMetadata, context?: Context): Promise<void>;
  ...
  
  // intents
  ...
  raiseIntent(intent: string, context: Context, app?: AppMetadata): Promise<IntentResolution>;
  raiseIntentForContext(context: Context, app?: AppMetadata): Promise<IntentResolution>;
  ...

  //deprecated
  open(name: String, context?: Context): Promise<void>;
  raiseIntent(intent: string, context: Context, name?: String): Promise<IntentResolution>;
  raiseIntentForContext(context: Context, name?: String): Promise<IntentResolution>;
}

@thorsent
Copy link
Contributor

Per today's discussion, I'd like to suggest that we break AppMetadata down:

AppIdentifier : {
   appId: string;
   instanceId ?: srtring
}
AppMetadata = AppIdentifier & {
   version ?: string;
   ...
}

This then allows inbound function signatures to accept AppIdentifier while outbound can deliver AppMetadata - but the AppMetadata can still be passed around the way Pierre had originally suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group channels feeds & transactions Channels, Feeds & Transactions Discussion Group enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants