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 metaswap and pooled staking events #717

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

casey-barr
Copy link
Contributor

What?

  • Added support for metaswap v1 and pooled staking contract addresses

How?

Related PRs (optional)

n/a

Anything Else?

n/a

Copy link
Collaborator

@kome12 kome12 left a comment

Choose a reason for hiding this comment

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

Just wanted to double check if you did the file names / table_names like this on purpose because it's usually camel cased.

}
],
"table_description": "",
"table_name": "metamask_swaps_v1_event_AdapterRemoved"
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the table_name, we usually camel case this (ie, in this case Metamask_Swaps_V1_event_AdapterRemoved).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no I can change this, thanks.

Copy link
Collaborator

@kome12 kome12 left a comment

Choose a reason for hiding this comment

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

Sorry, should have been more clear. Naming also applies to the pooled staking events as well. So something like Pooled_Staking_event_Deposited.

Also currently the CI/CD is failing because the file name doesn't match the table name.

Usually the v1 is actually lower cased (example would be AToken_v3_event_Approval

@casey-barr
Copy link
Contributor Author

Ah, gotcha @kome12! Thanks. I'll revise the whole thing then.

@casey-barr
Copy link
Contributor Author

Still a bit confused on the validate-json-files error...

@kome12
Copy link
Collaborator

kome12 commented Jun 4, 2024

Still a bit confused on the validate-json-files error...

Sorry about that, it's a bit confusing because I'm asking you to change quite a few things.

It's failing right now because the file name doesn't match with the table name. So if you changed the table name to be Pooled_Staking_event_MetadataUpdated, then the file name will also need to be Pooled_Staking_event_MetadataUpdated.json instead of pooled_staking_event_MetadataUpdated.json. It's case sensitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants