Skip to content
This repository was archived by the owner on Aug 30, 2022. It is now read-only.

Added New field Support#70

Merged
jakeloo merged 29 commits intomainfrom
new-fields
Dec 7, 2021
Merged

Added New field Support#70
jakeloo merged 29 commits intomainfrom
new-fields

Conversation

@ayshptk
Copy link
Copy Markdown
Contributor

@ayshptk ayshptk commented Dec 3, 2021

No description provided.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Dec 3, 2021

This pull request introduces 1 alert when merging 8b5b9b4 into f5d62b5 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

if (Buffer.isBuffer(object[keys[key]])) {
object[keys[key]] = await uploadToIPFS(object[keys[key]], contractAddress, signerAddress);
}
if (typeof object[keys[key]] === "object") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it possible to do || typeof File? @atbe

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have a type we can use here called FileOrBuffer that should be preferred.

(typeof object[keys[key]].name === "string" &&
typeof object[keys[key]] !== "object")
) {
object[keys[key]] = await uploadToIPFS(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this [keys[key]]?

@atbe
Copy link
Copy Markdown
Contributor

atbe commented Dec 7, 2021

Also I want to see the following tests:

  1. Uploading a File object (I think we can only do this in the browser, but I'm not sure). If it can only be tested in the browser, please verify the behavior within console. Also make sure the actions you take in console actually test the code paths we've modified.
  2. Uploading a Blob object
  3. Uploading recursive File || Blob properties. You already have this in the tests, but I want to see better verification instead of just checking the hash.

@ayshptk
Copy link
Copy Markdown
Contributor Author

ayshptk commented Dec 7, 2021

browser uploading works: tokenId is 4 in this contract

using v1.19.3-0

@jakeloo jakeloo merged commit 838663f into main Dec 7, 2021
@jakeloo jakeloo deleted the new-fields branch December 7, 2021 06:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants