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

feat: 🎸 Add Claim typeguards #668

Merged
merged 13 commits into from
Feb 3, 2022
Merged

Conversation

polymath-eric
Copy link
Collaborator

Add typeguard functions to cast Claims into specific types

BREAKING CHANGE: 🧨 isSingleClaimCondition, isMultiClaimCondition, isPortfolioCustodianRole, isVenueOwnerRole, isCddProviderRole, isTickerOwnerRole, isIdentityRole moved from src/types/index.ts to src/utils/typeguards.ts

Add typeguard functions to cast Claims into specific types

BREAKING CHANGE: 🧨 isSingleClaimCondition and isMultiClaimCondition moved from
`src/types/index.ts` to `src/utils/typeguards.ts`
BREAKING CHANGE: 🧨 Moved isPortfolioCustodianRole, isVenueOwnerRole, isCddProviderRole,
isTickerOwnerRole, isIdentityRole from `src/types/index.ts` to
`src/utils/typeguards.ts`
Copy link
Collaborator

@prashantasdeveloper prashantasdeveloper left a comment

Choose a reason for hiding this comment

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

LGTM

@prashantasdeveloper
Copy link
Collaborator

@monitz87 you can review this now

src/types/index.ts Outdated Show resolved Hide resolved
src/types/index.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
src/types/index.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
src/utils/typeguards.ts Outdated Show resolved Hide resolved
@polymath-eric
Copy link
Collaborator Author

Looks like the merge commit + auto format caused a bunch of brackets to be added in. Looking into it.

@polymath-eric
Copy link
Collaborator Author

polymath-eric commented Jan 27, 2022

Edit: The diff is cleaned up now.

So looks like the batch PR had some files with the newer prettier, but not all of them.

Made a PR for upgrading: https://github.com/PolymathNetwork/polymesh-sdk/pull/678/files so if that gets approved I can merge that in, and get rid of the white space noise here.

I don't think there is an easy way to clean up the noise with alpha only having it partially done.

@monitz87
Copy link
Contributor

monitz87 commented Feb 2, 2022

Other than the type/interface thing, LGTM

Convert claim types to use interfaces instead of types to be more inline
with the rest of the codebase

BREAKING CHANGE: 🧨 InvestorUniqueness, InvestorUniqunessV2, CddClaim converted from type to
interface
@sonarcloud
Copy link

sonarcloud bot commented Feb 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@polymath-eric polymath-eric merged commit dd86024 into alpha Feb 3, 2022
@polymath-eric polymath-eric deleted the MSDK-766-claim_typegaurds branch February 3, 2022 16:37
@monitz87
Copy link
Contributor

monitz87 commented Feb 3, 2022

🎉 This PR is included in version 13.0.0-alpha.19 🎉

The release is available on:

Your semantic-release bot 📦🚀

@monitz87
Copy link
Contributor

monitz87 commented Mar 4, 2022

🎉 This PR is included in version 14.0.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@monitz87
Copy link
Contributor

🎉 This PR is included in version 14.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@monitz87
Copy link
Contributor

🎉 This PR is included in version 14.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

None yet

3 participants