feat: npm package #7
Conversation
JUnit
|
# Conflicts: # src/handlers/generate-erc20-permit.ts
|
What does this PR change: No logic has been changed except adding an overload to be able to call this without needed a |
|
I don't have time to look through code, and will instead focus on communication. I think that permit requests make sense to make a library for, but actual permit generation should only happen in one place- and after our earlier discussion, perhaps from within the bot kernel. Due to this, I would expect that most of the logic to be related to "permit requests" from each planned plugin. |
|
@pavlovcik yep I agree. This library will simply allow us to move the logic and code outside to not pollute other repos and to avoid duplicating the code. Very handy currently because even if the generation is made only in the kernel, reading the actual permit can happen in multiple places, like currently in |
|
I noticed Knip is failing in this repo, I will fix this today. |
|
I fixed Knip configuration and opened an another pull request, should be green after merging. We can either merge this one first and fix any detected issues later, or merge #9 first . If Knip will be merged first we should be able to see if this one does not introduce any new regressions. |
|
@gentlementlegen as expected after merging there are a few conflicts that have to be resolved by rebase / merge against fresh development branch, perhaps I will be able to resolve them directly in the GitHub's web editor. |
# Conflicts: # package.json # src/handlers/generate-erc721-permit.ts # yarn.lock
…package # Conflicts: # package.json # src/handlers/generate-erc721-permit.ts # yarn.lock
|
I found a few issues and fixed them, it complains about jest types, I will revert this asap and we should be ready to merge. |
|
Knip kung-fu and Jest tests green (tests passing on branch locally). I see that pull_request_target workflow is used for tests, perhaps we need to change this to pull_request as pull_request_target uses base branch, not pr branch. |
@gitcoindev I changed the |
Sure, I will have a look today. I have just checked locally and tests work on my machine, I will compare and fix this. |
Add explicit jest imports in register wallet test. Add @jest/globals to dev dependencies. Add missing explicit jest imports. Add jest imports. Fix failing mocks.
|
@gentlementlegen I fixed tests, had to add explicit imports and fix a few types that compiler complained with TS2345 error. Knip is also green, and I think we can merge this now and continue rework in next prs. I leave @whilefoo to re-review, approve from his side and merge. |
|
Knip: the Sisyphusian task |
😄 |
Resolves #5