CityOfZion / props

Ecosystem props than can be used to enhance project functionality
5 stars 3 forks source link

CU-320r1g0 - Move metadapps files from the current repo to the props … #46

Closed luc10921 closed 1 year ago

luc10921 commented 1 year ago

…repository. Make the props TS SDK to have IconDapp included. Refactor props SDK to match IconDapp structure.

Things noted, but not addressed on this issue:

Props

IconDApps

EDIT: forgot to mention, but the newer version of neo3-boa has some bugs, so I had to change some of the smart contracts, so the nef generated is not the same anymore

lllwvlvwlll commented 1 year ago

Task linked: CU-320r1g0 Move metadapps files from the current repo to the props repository. Make the props TS SDK to have IconDapp included. Refactor props SDK to match IconDapp structure.

melanke commented 1 year ago

There isn't any documentation related to IconDapp or PriceDapp inside docs, neither for the contract or sdk. I think this is fine if the documentation needs to be manually created, because we can do this on another issue, I am more concerned if this should be auto-generated from source and the setup is missing.

melanke commented 1 year ago

PriceDapp doesn't have an SDK?

melanke commented 1 year ago
melanke commented 1 year ago

iterable return is breaking, because they changed the way it used to work;

No problem we already have an issue on neon-invoker to fix this and we are going to implement this on neo3-invoker as well

melanke commented 1 year ago

Please, create an issue for each of the points you mentioned on the PR description so we can fix it later, also, create an issue to improve the documentation.

luc10921 commented 1 year ago

There isn't any documentation related to IconDapp or PriceDapp inside docs, neither for the contract or sdk. I think this is fine if the documentation needs to be manually created, because we can do this on another issue, I am more concerned if this should be auto-generated from source and the setup is missing.

Some things are autogenerated, other aren't, I'll do the auto-generated ones in this issue.

PriceDapp doesn't have an SDK?

It doesn't, only the smart contract https://github.com/simplitech/meta-dapps/tree/main/price-dapp

There is a folder called icon_dapp inside sdk/tests that is also not very self-explanatory, we know its related to IconDapp but what is the responsibility of that files, might be a good idea to wrap it inside another folder that explains this.

The folder icon_dapp has some additional smart contracts and a batch file to deploy them to test some operations on the SDK test file, the props' tests doesn't have additional contracts for testing. Do you think making something like:

props
↳sdk
  ↳tests
    ↳smart_contracts
      ↳icon_dapp
        child.py
        ownership.py
        parent.py
    ↳batch_files
      ↳icon_dapp
        initialize.batch
      ↳price_dapp
        initialize.batch
    Collection.spec.ts
    Dice.spec.ts
    helper.ts
    IconDApp.spec.ts
    Puppet.spec.ts

would be better?

I think the folder price_dapp inside sdk/tests is unnecessary

It may be now, but the batch file should be useful when the SDK is implemented. (It was also used for testing the smart contract)

melanke commented 1 year ago

Do you think making something like

Yes

melanke commented 1 year ago

It would be interesting to have the constants for MAINNET and TESTNET for the scripthash, just like Letter

luc10921 commented 1 year ago

I created those issues on clickup