PRO-2111- Config Changes #65
Conversation
vignesha22
requested review from
ch4r10t33r,
IAmKio,
poocart and
0xSulpiride
Deploying with
|
| Latest commit: |
d936934
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://14722d5b.arka-3qg.pages.dev |
| Branch Preview URL: | https://pro-2111-config-changes.arka-3qg.pages.dev |
There was a problem hiding this comment.
A few things still outstanding but much better than before:
- In the admin_frontend code indentation still isn't aligned with the rest of the repo (tabs vs spaces)
- About new lines: https://thoughtbot.com/blog/no-newline-at-end-of-file, i didn't see a .editorconfig file committed
- The routes file is becoming huge, this should be split out into controllers in the future
- Consider a defacto logger like Winston in the future, i saw some custom logging function but wasn't sure what it's using behind the scenes
| @@ -85,9 +106,10 @@ const ApiKeysPage = () => { | |||
| toast.error("Could not save"); | |||
There was a problem hiding this comment.
Can you give extra instructions to what the user should do next?
There was a problem hiding this comment.
have to see the logs on backend on what's causing the error
There was a problem hiding this comment.
should I give that on the toast?
There was a problem hiding this comment.
Toast is okay, but for now you can just ask them to check their details and try again, or contact the support team.
| 'getKeys': '/getKeys', | ||
| 'saveKey' : '/saveKey', | ||
| 'deleteKey': '/deleteKey', | ||
| } |
There was a problem hiding this comment.
New line missing - did you add this to your editor config?
There was a problem hiding this comment.
yes on the other PR and when I changed the branch it got deleted
| /.ponder/ | ||
|
|
||
| package-lock.json | ||
|
|
There was a problem hiding this comment.
Missing new line, this can be done automatically with editorconfig
There was a problem hiding this comment.
line 21 is a new line
| "stateMutability": "nonpayable", | ||
| "type": "function" | ||
| } | ||
| ] as const; |
There was a problem hiding this comment.
Still not comfortable with a JSON file being cast as s JS object and exported in a TS file... it's just a JSON file
There was a problem hiding this comment.
yes but the import through assertion, the ponder does not take it and moreover the ponder docs have the same type
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)