ubiquibot / permit-generation

A standalone module to generate permits.
0 stars 6 forks source link

feat: generate erc20 / erc721 permit #4

Closed gentlementlegen closed 3 months ago

gentlementlegen commented 3 months ago

Based on PR https://github.com/ubiquibot/permit-generation/pull/2 Aims to enable ERC 20 / 721 permit generation.

github-actions[bot] commented 3 months ago
Coverage Report (0%) 
File% Stmts% Branch% Funcs% LinesUncovered Line #s
0x4007 commented 3 months ago

Make sure to file an issue to handle erc721 stuff later. It's not a top priority and we are behind schedule.

molecula451 commented 3 months ago

ready to review? @gentlementlegen

gentlementlegen commented 3 months ago

@molecula451 was trying to get tests working first but they are using bun which doesn't play well with jest imports. Review should be ready, I'll try to get a run on my repo and link it here instead.

A successful run can be found at https://github.com/gentlementlegen/permit-generation/actions/runs/8544054410/job/23409249851#step:4:84

gentlementlegen commented 3 months ago

Thanks for the review team. Latest run: https://github.com/gentlementlegen/permit-generation/actions/runs/8549616506/job/23425247355

0x4007 commented 3 months ago

Seems good enough. Lets test in "production" aka lets run the kernel without any real funds, concurrently, with our old bot instance to compare performance/results.

When this is merged, will the CI be fixed?

gentlementlegen commented 3 months ago

@pavlovcik Yep CI should be, because right now CI is running with the file contained within the repo development branch not with my changes on this branch.

0x4007 commented 3 months ago

@molecula451 @gitcoindev please review as soon as possible! Thank you.

gentlementlegen commented 3 months ago

New latest run after changes https://github.com/gentlementlegen/permit-generation/actions/runs/8566692541/job/23476937894

molecula451 commented 3 months ago

i thought gitcoindev had fixed knip? why no deprecated the feature if it's not going to fix?

gitcoindev commented 3 months ago

i thought gitcoindev had fixed knip? why no deprecated the feature if it's not going to fix?

hi @molecula451 Knip pull request was not merged yet, I added you to reviewers: https://github.com/ubiquity/keygen.ubq.fi/pull/5#pullrequestreview-1982357342 . First Knip fix goes to keygen repo, then after approval I will clone and fix to all other repositories.