finos / FDC3

An open standard for the financial desktop.
https://fdc3.finos.org
Other
202 stars 132 forks source link

Support broadcast in FDC3 Action #1368

Closed symphony-jean-michael closed 1 month ago

symphony-jean-michael commented 2 months ago

This PR applies the changes requested in the Enhancement Request: https://github.com/finos/FDC3/issues/1353

linux-foundation-easycla[bot] commented 2 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

netlify[bot] commented 2 months ago

Deploy Preview for fdc3 ready!

Name Link
Latest commit 4f34a715a9736bcfdfa6aa7577efdbe8d057b92c
Latest deploy log https://app.netlify.com/sites/fdc3/deploys/670cee97044cae000887c496
Deploy Preview https://deploy-preview-1368--fdc3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

symphony-jean-michael commented 2 months ago

Hi @kriswest Thank you for your help. I've modified the PR according to your comments. Now only the action.schema.json file has been updated.

symphony-jean-michael commented 1 month ago

@kriswest Thank you for your help. I committed your suggestion.

kriswest commented 1 month ago

Hi @symphony-jean-michael, looking good now - just needs a changelog entry adding: https://github.com/symphony-jean-michael/FDC3/blob/broadcast_in_action/CHANGELOG.md

I would suggest something like the following (in the 'Added' section):

- Added support for broadcast actions to the `fdc3.action` context type, allowing an Action to represent the broadcast of a specified context to an app or user channel. ([#1368](https://github.com/finos/FDC3/pull/1368))

You could also run npm run build in the root of the project before checking in so that ContextTYpes.ts gets regenerated.

We're going to add a PR template to the project soon with checkboxes for things like this to avoid slow back and forth on reviews like this ;-)

symphony-jean-michael commented 1 month ago

Hi @kriswest I updated the Changelog file. :)

kriswest commented 1 month ago

Hi @kriswest I updated the Changelog file. :)

Don't forget to push the change (not seeing it yet - but then github seems to be under particularly high load this morning so it may be them). Please add /src/ContextTypes.ts as well (should have changes after running npm run build).

symphony-jean-michael commented 1 month ago

@kriswest Yes it seems to take time. 😅 Screenshot 2024-10-14 at 11 40 58