(chore): update developer documentation #25

Jipperism · 2024-12-14T17:31:18Z

Updates developer documentation for contributing to the sdk.
Updates developer documentation for consuming the SDK.
Removes redundant references to env vars.
Removes redundant references to outdated package manager yarn.
Updates typedoc version and configuration to include handwritten guides in the generated docs.
Removes outdated github templates.
Updates JSDoc annotations.
Bump version number.

.vscode/settings.json

CONTRIBUTING.md

doc/_media/cancelOrders.md

pheuberger

Looks like you're not only removing old code, but also bumping the version number and changing some magic number. I would advocate for even more granular commits, because I don't know why you're doing any of that (except the version bump).

pheuberger

Great stuff. Thank you so much for making this! It's mostly just nits and a couple of clarifying questions from my side. Being new to the project, I have big gaps in my knowledge and understanding which shows in my questions. Feel free to not elaborate on some of them if you think they're just too preposterous to cover hehe.

Please also don't be put off by the amount of comments. I was very thorough and highlighted very minor things. Generally this is great!

src/utils/api.ts

.gitignore

README.md

guides/orderValidity.md

pheuberger · 2024-12-19T16:00:30Z

src/HypercertExchangeClient.ts

@@ -351,7 +319,7 @@ export class HypercertExchangeClient {
   * @param maker Maker order
   * @param taker Taker order
   * @param signature Signature of the maker order
-   * @param merkleTree If the maker has been signed with a merkle tree
+   * @param merkleTree Optional merkle tree


Why would the user use a Merkle tree? I think that would be a much better parameter description.

I had a more in-depth look, and I originally misinterpreted what the merkleTree was referring to. It seems to only refer to the proofs that are obtained when using signMultipleMakerOrders() to create the orders. See https://docs.looksrare.org/developers/sdk/signMultipleMakerOrders for a more thorough explanation.

I thought we would use this for allowlist orders, but they are implemented using a strategy and the merkle tree proofs for the allowlist are provided using the additionalParams argument (contract implementation: https://github.com/hypercerts-org/hypercerts/blob/main/contracts/src/marketplace/executionStrategies/StrategyHypercertFractionOffer.sol#L169). So it looks like we will only need this in the foreseeable future, if we plan on supporting signing multiple sales (and buys) at once from the frontend. That would also include implementing the API endpoint for storing the tree and retrieving proofs (see https://docs.looksrare.org/developers/sdk/signMultipleMakerOrders again).

We could decide to remove the functionality, and that means we can get rid of all merkleTree parameters in HypercertExchangeClient. It would be a breaking change however, as the function signatures would change, but I think it might be worth it.

Looping in @bitbeckers and @holkexyz to weigh in on whether we are going to need this functionality in the future.

Thank you for the thorough explanation! Also waiting for @bitbeckers and @holkexyz to chime in here.

I wouldn't invest time in these kinda of refactors. Is we're going to invest significant amounts of time in the marketplace SDK it would be migrate it to viem in the regular SDK.

src/HypercertExchangeClient.ts

src/utils/api.ts

typedoc.json

Updates outdated strategy IDs. As we aren't using hypercertFractionAllowlist strategy currently, it wasn't noticed that it's outdated. This fixes that already for when we decide to start using it. Removes outdated code that's related to: - Batch transfers - Subset nonces - Remove amounts argument when creating a makerask, as in the case of hypercerts it will always be an array of 1's. - Bump version number

Updates developer documentation for contributing to the sdk. Updates developer documentation for consuming the SDK. Removes redundant references to env vars. Removes redundant references to outdated package manager yarn. Updates typedoc version and configuration to include handwritten guides in the generated docs. Removes outdated github templates. Updates JSDoc annotations.

Jipperism · 2024-12-26T14:13:20Z

@pheuberger Had a look at all the comments and implemented the feedback. Please close all conversations on the PR that you consider resolved.

Looks like you're not only removing old code, but also bumping the version number and changing some magic number. I would advocate for even more granular commits, because I don't know why you're doing any of that (except the version bump).

I updated the git commit description for fe6ed4b, hopefully that makes things clearer. I think splitting this up in more granular commits would also entail purposely releasing broken versions to NPM, as each commit would require a version bump (otherwise we have version on github that don't exist on NPM)? I can see both sides of the story but maybe that's not necessary for now, let me know what you think.

README.md

guides/cancelOrders.md

pheuberger · 2025-01-03T11:45:42Z

typedoc.json

  "out": "./doc",
  "githubPages": false,
  "readme": "none",
  "gitRemote": "https://github.com/hypercerts-org/marketplace-sdk",
  "disableSources": true,
  "cleanOutputDir": false,

-  "hideBreadcrumbs": true,
-  "hideInPageTOC": true
+  "projectDocuments": ["./guides/readme.md"],


This one should also be all caps like above, shouldn't it?

pheuberger · 2025-01-03T12:00:40Z

@pheuberger Had a look at all the comments and implemented the feedback. Please close all conversations on the PR that you consider resolved.

Looks like you're not only removing old code, but also bumping the version number and changing some magic number. I would advocate for even more granular commits, because I don't know why you're doing any of that (except the version bump).

I updated the git commit description for fe6ed4b, hopefully that makes things clearer. I think splitting this up in more granular commits would also entail purposely releasing broken versions to NPM, as each commit would require a version bump (otherwise we have version on github that don't exist on NPM)? I can see both sides of the story but maybe that's not necessary for now, let me know what you think.

Thank you, @Jipperism! I closed all the conversations that are implemented/answered :)

Can you explain why we would need to release broken versions to NPM? Not every commit needs to be a release, nor have a version bump, unless I'm missing something?

pheuberger · 2025-01-22T13:54:20Z

README.md

+cd marketplace-sdk
+pnpm install && pnpm run build
+cd ../your-project
+pnpm link @hypercerts-org/marketplace-sdk


This is actually wrong. It needs to say

pnpm link <path to marketplace sdk on you local filesystem>

Jipperism mentioned this pull request Dec 14, 2024

(chore): update developer documentation #24

Closed

Jipperism self-assigned this Dec 14, 2024

Jipperism requested review from pheuberger and bitbeckers December 14, 2024 17:32

Jipperism force-pushed the feature/developer-docs branch 4 times, most recently from b1c8fa0 to aff056d Compare December 14, 2024 17:48

bitbeckers requested changes Dec 16, 2024

View reviewed changes

.vscode/settings.json Outdated Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

doc/_media/cancelOrders.md Outdated Show resolved Hide resolved

Jipperism force-pushed the feature/developer-docs branch 20 times, most recently from f37c9fe to c75bfff Compare December 19, 2024 13:44

pheuberger reviewed Dec 19, 2024

View reviewed changes

Jipperism added 3 commits December 26, 2024 10:13

(chore): update gitignore to include vscode configuration files

cbc28cc

Jipperism force-pushed the feature/developer-docs branch from c75bfff to cbc28cc Compare December 26, 2024 14:08

pheuberger reviewed Jan 3, 2025

View reviewed changes

README.md Show resolved Hide resolved

README.md Show resolved Hide resolved

guides/cancelOrders.md Show resolved Hide resolved

pheuberger reviewed Jan 3, 2025

View reviewed changes

pheuberger reviewed Jan 22, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(chore): update developer documentation #25

(chore): update developer documentation #25

Jipperism commented Dec 14, 2024 •

edited

Loading

pheuberger left a comment

pheuberger left a comment •

edited

Loading

pheuberger Dec 19, 2024

Jipperism Dec 26, 2024 •

edited

Loading

pheuberger Jan 3, 2025

bitbeckers Jan 22, 2025

Jipperism commented Dec 26, 2024

pheuberger Jan 3, 2025

pheuberger commented Jan 3, 2025 •

edited

Loading

pheuberger Jan 22, 2025

(chore): update developer documentation #25

Are you sure you want to change the base?

(chore): update developer documentation #25

Conversation

Jipperism commented Dec 14, 2024 • edited Loading

pheuberger left a comment

Choose a reason for hiding this comment

pheuberger left a comment • edited Loading

Choose a reason for hiding this comment

pheuberger Dec 19, 2024

Choose a reason for hiding this comment

Jipperism Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

pheuberger Jan 3, 2025

Choose a reason for hiding this comment

bitbeckers Jan 22, 2025

Choose a reason for hiding this comment

Jipperism commented Dec 26, 2024

pheuberger Jan 3, 2025

Choose a reason for hiding this comment

pheuberger commented Jan 3, 2025 • edited Loading

pheuberger Jan 22, 2025

Choose a reason for hiding this comment

Jipperism commented Dec 14, 2024 •

edited

Loading

pheuberger left a comment •

edited

Loading

Jipperism Dec 26, 2024 •

edited

Loading

pheuberger commented Jan 3, 2025 •

edited

Loading