nft-gallery icon indicating copy to clipboard operation
nft-gallery copied to clipboard

Remove giant squid from the code

Open Jarsen136 opened this issue 9 months ago • 9 comments

Thank you for your contribution to the Koda - Generative Art Marketplace.

👇 __ Let's make a quick check before the contribution.

PR Type

  • [x] Bugfix
  • [x] Refactoring

Needs QA check

  • @kodadot/qa-guild please review

Context

  • [x] Closes #10268
  1. replace on-chain identity with profile data
  2. remove giant squid request
  3. remove the identity setting page completely
  4. performance: batch request several profiles data in one request https://github.com/kodadot/private-workers/pull/156
  5. Clean the console log for profile request https://github.com/kodadot/private-workers/pull/159

Did your issue had any of the "$" label on it?

Screenshot 📸

  • [x] My fix has changed something on UI; a screenshot is best to understand changes for others.

image

image

Jarsen136 avatar May 14 '24 20:05 Jarsen136

Deploy Preview for koda-canary ready!

Name Link
Latest commit 6aea0b66c04f6241a22fda9cf44b70cb6c55e8b0
Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/664a7f6faddf320008835093
Deploy Preview https://deploy-preview-10299--koda-canary.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar May 14 '24 20:05 netlify[bot]

Code Climate has analyzed commit bcce23e and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count Duplication 2 View more on Code Climate.

It's the wrong analysis from codeclimate. Let's ignore it.

Jarsen136 avatar May 17 '24 13:05 Jarsen136

tests can be ignored as well, twitter changed itself to x

prury avatar May 17 '24 13:05 prury

i'm checking the other errors

prury avatar May 17 '24 14:05 prury

getting some different errors now:

image

image

prury avatar May 17 '24 16:05 prury

getting some different errors now:

not sure errors is the right word here

these profiles simply don't exist

daiagi avatar May 17 '24 16:05 daiagi

getting some different errors now:

not sure errors is the right word here

these profiles simply don't exist

Yup. It's the expected response result with a 404 status code, so it's not a real "error".

However, if we would like to make the console look more clean without showing any request 404 code, I will simply change the 404 code to 200 and return null value. Only applied for API of /profile/:address because it should be called lots of times. In this way, we could "hide" the 404 request error from the console.

https://github.com/kodadot/private-workers/pull/159

Jarsen136 avatar May 17 '24 21:05 Jarsen136

getting some different errors now:

not sure errors is the right word here these profiles simply don't exist

Yup. It's the expected response result with a 404 status code, so it's not a real "error".

However, if we would like to make the console look more clean without showing any request 404 code, I will simply change the 404 code to 200 and return null value. Only applied for API of /profile/:address because it should be called lots of times. In this way, we could "hide" the 404 request error from the console.

kodadot/private-workers#159

✅ https://github.com/kodadot/private-workers/pull/159 Deployed and also removed the error log from the console

Jarsen136 avatar May 18 '24 13:05 Jarsen136

Code Climate has analyzed commit 6aea0b66 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

View more on Code Climate.

codeclimate[bot] avatar May 19 '24 22:05 codeclimate[bot]

can benefit from caching

rest is kinda minor

Thanks! I have updated them.

Jarsen136 avatar May 19 '24 22:05 Jarsen136

@Jarsen136 you may want to look at #10323

daiagi avatar May 20 '24 06:05 daiagi

@Jarsen136 you may want to look at #10323

https://github.com/kodadot/nft-gallery/issues/10324 It could be done first.

Jarsen136 avatar May 20 '24 08:05 Jarsen136

@vikiival How about merging it ?

Jarsen136 avatar May 21 '24 08:05 Jarsen136

Is it somehow cached?

vikiival avatar May 21 '24 08:05 vikiival

Is it somehow cached?

image

It's already cached but not ideal. Ideally, It needs a better fetching library with cache. Should I implement it at the current PR also? https://github.com/kodadot/nft-gallery/discussions/10329

Jarsen136 avatar May 21 '24 10:05 Jarsen136