finos / FDC3

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

Add event listener support to the Desktop Agent API #1207

Closed kemerava closed 5 days ago

kemerava commented 1 month ago

Closes #1136 Adding support for event listener for non-context and non-intent events @michael-bowen-sc

linux-foundation-easycla[bot] commented 1 month ago

CLA Signed

The committers listed above are authorized under a signed CLA.

netlify[bot] commented 1 month ago

Deploy Preview for fdc3 ready!

Name Link
Latest commit 3fdc6fd20eafebd77944479ead39f2619f4cbbae
Latest deploy log https://app.netlify.com/sites/fdc3/deploys/66561d43352d7b000846eec6
Deploy Preview https://deploy-preview-1207--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.

kemerava commented 1 month ago

Thank you for the review, @kriswest! I am working on trying to figure out why we are getting the error from the CCLA side, as we have indeed signed the CCLA already. Regarding that commit with missing github id, I was able to resolve that, and the error message went away. Thank you for the suggestions, I have applied them, however, right after that easyCLA posted the same issue as before with the missing id. On the commit it has just you and I as authors: Screenshot 2024-05-17 at 1 57 06 PM Do you know why this is causing an issue, as both of our handles seem to be recognized?

kriswest commented 1 month ago

I am working on trying to figure out why we are getting the error from the CCLA side, as we have indeed signed the CCLA already.

If the CCLA is in place, you still need to click on the Please click here to be authorized link - that will confirm your status on the CCLA and future updates won't require it. In case you've already done that, I'll see if I can get easyCLA to recheck via a comment - failing that we may have to ask their support for help (or further assistance with EasyCLA, please submit a support request ticket.)

Glad you got the other error resolved!

kriswest commented 1 month ago

/easycla

kemerava commented 1 month ago

Thank you, @kriswest! I created the support issue for the github account easyCLA error: https://jira.linuxfoundation.org/plugins/servlet/desk/portal/4/SUPPORT-26917

Thanks for the info on the CCLA part, I will bring it back to my team to let them know

kriswest commented 1 month ago

I opened one as well @kemerava: https://jira.linuxfoundation.org/plugins/servlet/desk/portal/4/SUPPORT-26910

They may know what's up... but also made a comment that suggests you haven't yet hit the 'click here to be authorized' link in the comment the EasyCLA bot made above and selected proceed as a corporate contributor and been approved - if you have, add a comment here so they see it.

kriswest commented 1 month ago

Also if you visit: https://patch-diff.githubusercontent.com/raw/finos/FDC3/pull/1207.patch you'll see the emial it is picking up for you (which may not be the right one to get you approved on a corporate CLA)

kriswest commented 1 month ago

/easycla

kriswest commented 1 month ago

@kemerava they rolled back easyCLA and it is running checks again properly - it cleared one of the errors so I know its updated. Now it just needs to know which CCLA you are on - so go hit that Please click here to be authorized link and complete the steps. If you still get nowhere then maybe add your work email to your github account if you haven't done so already - the email domain is often used as the approval criteria by CLA managers. Alternatively, your firm's CLA manager can manually add you to the CCLA.

kemerava commented 1 month ago

Hi @kriswest,

Just finished figuring out the CLA from our side. Looks like there are no more errors from the easyCLA side. Please let me know what else is required from my side for this PR. Thank you for all your help!

kriswest commented 1 month ago

@kemerava looks good to me - we got consent at today's meeting to go ahead with this once it passes a couple of reviews, which will hopefully happen over the next couple of weeks.

Again, many thanks for the contribution!

kriswest commented 1 month ago

LGTM and matches the proposal. Applied your suggestions @hughtroeger after figuring out that you need to use quadruple backticks in github suggestions involving triple backticks!

I was involved in the proposal, so adding other maintainers to review and will leave merging a week or two to facilitate.

kemerava commented 3 weeks ago

Hi @kriswest, any updates on this? Anything needed from my side? Thank you!

kriswest commented 3 weeks ago

Hi @kemerava, nothing more is needed apart from a review from another maintainer (which it looks like I forgot to request - done now)!

kemerava commented 1 week ago

Hi @kriswest, Just checking in here, are there any more approvals required for this? Thank you so much for helping with this PR! Elizabeth

michael-bowen-sc commented 5 days ago

Hi @kriswest, Just checking in here, are there any more approvals required for this? Thank you so much for helping with this PR! Elizabeth

Looks like this is all green. What is holding this PR up from merge?

kriswest commented 5 days ago

@michael-bowen-sc now that Brian and Vinay have had a look it's all good to go. Apologies for the delay, OSFF prep was.distractong me!