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

Allow AppMetadata to be passed in as a target argument #272

Merged
merged 3 commits into from
Nov 13, 2020

Conversation

sean-ciq
Copy link
Contributor

@sean-ciq sean-ciq commented Oct 21, 2020

This PR contributes changes proposed in #270

  • Changes the name argument in open() to target and adds AppMetadata as a valid type for that argument
  • Adds AppMetadata as a valid type for the target argument in raiseIntent()
  • Updates documentation

src/api/DesktopAgent.ts Outdated Show resolved Hide resolved
@mattjamieson
Copy link
Contributor

I suggest we create:

type Target = string | AppMetadata;

And update raiseIntent (and the new raiseIntentForContext):

raiseIntent(intent: string, target: Target) : Listener;

Copy link
Contributor

@kriswest kriswest left a comment

Choose a reason for hiding this comment

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

WG feedback - merge this and raise a separate PR to introduce a type for target (see @mattjamieson comment)

@mattjamieson
Copy link
Contributor

Sorry, in with another comment at the last minute. If we're updating target on raiseIntent and open, should we also update IntentResolution.source to be consistent?

@sean-ciq
Copy link
Contributor Author

I have created a new PR for @mattjamieson's proposed edits #279

@nkolba nkolba mentioned this pull request Nov 13, 2020
8 tasks
@rikoe rikoe merged commit 135aed3 into finos:master Nov 13, 2020
@rikoe rikoe added this to the 1.2 milestone Jan 26, 2021
@kriswest kriswest deleted the feature/enhance-target-argument branch October 8, 2021 11:38
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.

4 participants