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

JEAN BAPTISTE ZIADE removeAbusiveConditions-PerformancePayout #3040

Draft
wants to merge 1 commit into
base: 5.x.x
Choose a base branch
from

Conversation

regnosys-prod-user
Copy link
Collaborator

No description provided.

removeAbusiveConditions-PerformancePayout
Copy link

netlify bot commented Jul 12, 2024

Deploy Preview for finos-cdm ready!

Name Link
🔨 Latest commit 58598bb
🔍 Latest deploy log https://app.netlify.com/sites/finos-cdm/deploys/66912cc23c3da700083f403e
😎 Deploy Preview https://deploy-preview-3040--finos-cdm.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.

@JBZ-Fragmos
Copy link
Contributor

business need to remove these 2 conditions that should not have been implemented at the time when we (mostly Fragmos-Chain based on JPM business case) designed the PortfolioReturnTerms and attached it to PerformancePayout also with such abusive conditions.

image

rationale why removing - examples, business cases :

  • blockers when at some point in lifecycle a Portfolio might involve only 1 underlier in position (that is to say when only 1 unique PortfolioReturnTerms leg may exist at some point in time, which is not permitted with current condition)

image

  • prevent larger usage of PortfolioReturnTerms - for instance for representing Dispersion Strategy involving multiple individual components vs. single index component (no Basket is involved in such case, so current conditions would prevent it, though PortfolioReturnTerms was notabky designed to encompass this type of strategy)

image

@JBZ-Fragmos
Copy link
Contributor

@dshoneisda @lolabeis @nicholas-moger

i assume such minor fix can be pushed without requiring discussion/validation with the Group, right ?

@dshoneisda
Copy link
Contributor

@lolabeis @Oblongs please can you review this item on my behalf

@LionelSG-REGnosys
Copy link
Contributor

@lolabeis @Oblongs please can you review this item on my behalf

The change itself looks relatively straightforward - removing two conditions on one data type - however this pull request seems to be corrupt as there are 1600+ files recorded as having been changed. This needs to be fixed.

@dshoneisda
Copy link
Contributor

thanks
@JBZ-Fragmos can you look at the multiple file issue/re-submit if necessary

@JBZ-Fragmos
Copy link
Contributor

JBZ-Fragmos commented Jul 16, 2024 via email

@LionelSG-REGnosys
Copy link
Contributor

LionelSG-REGnosys commented Jul 16, 2024

looks like something technical beyond my scope

Have raised to the team to investigate

Update Has been fixed

@hugohills-regnosys hugohills-regnosys changed the base branch from master to 5.x.x July 16, 2024 12:57
@JBZ-Fragmos
Copy link
Contributor

JBZ-Fragmos commented Sep 18, 2024

@Oblongs @lolabeis @dshoneisda

reading comment history about this proposal, also considering it is really minor technical one, which requires only 1 approver, kindly let me know if this could be pushed to Prod in coming days

thanks
JB

@JBZ-Fragmos
Copy link
Contributor

JBZ-Fragmos commented Nov 7, 2024

Background :

Target :

  • merge any agreed components into Dev, before next major release that will incorporate such changes into Prod

@JBZ-Fragmos JBZ-Fragmos added Status: needs review Needs to be reviewed by Maintainer Target: Development and removed Target: Production labels Nov 7, 2024
@JBZ-Fragmos
Copy link
Contributor

@Oblongs @lolabeis @dshoneisda

reading comment history about this proposal, also considering it is really minor technical one, which requires only 1 approver, kindly let me know if this could be pushed to Prod in coming days

thanks
JB

@LionelSG-REGnosys
Copy link
Contributor

LionelSG-REGnosys commented Nov 11, 2024 via email

@JBZ-Fragmos
Copy link
Contributor

hello, @lolabeis @dshoneisda @hugohills

Could you please review this PR ?
that is a technical one in my view, so should not need review with DPBE, just need 1 approval...

FYI, i regret having myself drafted/introduced these conditions at the time had designed the "portfolio" component wihout nobody asking about such conditions to exist, and now i'm asking to remove them, after having clarified with JPM also in regards in some particular business cases that these conditions are actuallu restricting business usage of the "portofolio" component...

@manel-martos
Copy link
Contributor

Hi @JBZ-Fragmos ,

We’ve reviewed the changes and confirmed that they do not impact the CDM product qualification logic or the DRR reporting model.

Given the minor technical nature of the change and its alignment with the clarified business requirements, we’re happy to approve it. Could you please confirm whether this should be applied to the latest CDM 5 production version, or if it suffices to be included in the latest development version?

@JBZ-Fragmos
Copy link
Contributor

JBZ-Fragmos commented Jan 22, 2025

@manel-martos thanks for your feedback, no need to implement in Prod, moreover knowing that Dev 6 will become the next Prod in few days, so kindly release this in Dev 6

@LionelSG-REGnosys
Copy link
Contributor

It won't make CDM6-dev now, as that is being prepared for release. But when we have CDM6-prod, this item can be lined up as release to that.

Having said that, you will need to update the logic in the conditions to work with the new underlier in PerformancePayout. It may be cleaner to prepare a new Contribution in Rosetta?

@LionelSG-REGnosys
Copy link
Contributor

Sorry, accidentally clicked wrong button and closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Technical/Patch Individual Maintainer May Approve DerivsWG Rosetta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants