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: wip merkl report type #274

Closed
wants to merge 1 commit into from

Conversation

greedythib
Copy link
Member

No description provided.

@greedythib greedythib self-assigned this Oct 5, 2023
[holder: string]: {
[poolAddress: string]: {
details: {
[rewardId: string]: {
Copy link
Contributor

Choose a reason for hiding this comment

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

I liked the sub types too, because sometimes you only work with the sub object and you don't want to retype it

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that for a start, it helps to have an overview of the type
once in prod, I agree that segmenting into smaller types is better

};
};

export type MerklReportType = Partial<{
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this one corresponding too? I don't see the same params as in merkl-dispute

Copy link
Member Author

Choose a reason for hiding this comment

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

it was inspired from MerklReport


import { AggregatedRewardsType, MerklSupportedChainIdsType } from '.';

export type OnChainParamsType = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it a convention to add Type at the end of the name?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it's a convention for clarity and readability

@greedythib greedythib closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants