Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Data intents #21

Closed
wants to merge 5 commits into from
Closed

Data intents #21

wants to merge 5 commits into from

Conversation

nkolba
Copy link
Contributor

@nkolba nkolba commented Nov 22, 2020

This is a demo implementation of AppInstances and supporting data intents, as proposed in PR #287 to FDC3. Highlights include:

  • Addition of AppInstance class
  • Addition of getAppInstance method to the DesktopAgent api
  • Addition of source arg to ContextHandler
  • Support for direct routing of events via broadcast and addContextListener using AppInstance
  • Support for last value cache of context on AppInstance

Still to do:

  • full lifecycle support
  • clean up bring to front handling

@nkolba nkolba requested a review from rikoe November 22, 2020 14:49
@finos-cla-bot
Copy link

finos-cla-bot bot commented Nov 22, 2020

Thank you for your contribution and Welcome to our Open Source Community!

To make sure your pull request is successful, we need all our contributors to be identifiable, but we couldn't parse the GitHub details of the following people : Nicholas Kolba

Luckily, resolving the issue is straightforward and you can resolve it by following the instructions below.

  1. Check your git client is configured with a user email git config --list | grep email
  2. If the user email is missing, run the following command, substituting with your git commit email address git config --global user.email email@example.com
  3. Make sure your git commit email is configured on GitHub by Setting your Commit Email Address
  4. Then, amend the authors in your commit history by using git commit --amend to change your last commit.

Alternatively, use the slightly more complex git reset --soft and git rebase to checkout your changes, rewrite the commit history locally and (force) push changes to the downstream branch.

If you have any issues with the steps above, please email help@finos.org so we can help you resolve before reviewing and accepting your pull request.

Thanks once again for the contribution and understanding.

cc @finos-admin

@finos-cla-bot
Copy link

finos-cla-bot bot commented Nov 22, 2020

Thank you for your contribution and Welcome to our Open Source Community!

To make sure your pull request is successful, we need all our contributors to be identifiable, but we couldn't parse the GitHub details of the following people : Nicholas Kolba

Luckily, resolving the issue is straightforward and you can resolve it by following the instructions below.

  1. Check your git client is configured with a user email git config --list | grep email
  2. If the user email is missing, run the following command, substituting with your git commit email address git config --global user.email email@example.com
  3. Make sure your git commit email is configured on GitHub by Setting your Commit Email Address
  4. Then, amend the authors in your commit history by using git commit --amend to change your last commit.

Alternatively, use the slightly more complex git reset --soft and git rebase to checkout your changes, rewrite the commit history locally and (force) push changes to the downstream branch.

If you have any issues with the steps above, please email help@finos.org so we can help you resolve before reviewing and accepting your pull request.

Thanks once again for the contribution and understanding.

cc @finos-admin

/**
* Adds a listener for incoming contexts whenever a broadcast happens from this instance.
*/
addContextListener(handler: ContextHandler): Listener;
Copy link

Choose a reason for hiding this comment

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

Why have overloading for the same method name rather than calling this something like addBroadcastListener? Having a dedicated method for the separate context makes the developer intent clear and less error-prone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

excellent question. I think the distinction you are making is listening for a specific context type (addContextListener) and listening for any broadcast (addBroadcastListener)?

The API is coming from the FDC3 standard - and has evolved somewhat over time - so there is definitely opportunity for adjustment.

@nkolba nkolba closed this Dec 21, 2020
@nkolba
Copy link
Contributor Author

nkolba commented Dec 21, 2020

closed due to cla-bot issues - will reopen.

@nkolba nkolba deleted the data-intents branch December 21, 2020 12:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants