Update readme for client and other packages by krisbitney · Pull Request #1457 · polywrap/wrap-cli

krisbitney · 2022-12-20T15:59:14Z

This is a draft PR for the purpose of preparing readme documents for the 0.10.0 release. The readme docs will be used for reference documentation on our docs site.

Steps to update a typical readme with reference documentation:

If a readme does not exist, create one with an introduction, some usage examples, etc.
Add doc comments to exported methods and types.
Add doc-snippets tokens around the comments and types or method signatures.
Use the doc-snippets CLI tool to inject the snippets into the readme doc.

You can copy the doc-snippets setup and configuration from repos where this has been done.

The aforementioned steps won't make sense for every package. Feel free to adapt as necessary!

TODO:

pileks

Overall LGTM! Comment is non-blocking.

pileks · 2022-12-20T18:10:00Z

+  resolvers: [
+    new RecursiveResolver(
+      new PackageToWrapperCacheResolver(wrapperCache, [
+        new ExtendableUriResolver(),


Do we have any docs that explain the various resolvers, at least the ones that come with Polywrap by default?

This might help developers set up their own resolution logic.

I'm thinking we could add this to packages/js/uri-resolver-extensions/README.md and reference it here? @nerfZael thoughts?

Imo that's a good idea. I can work on it soon. I was also thinking to write more detailed reference documentation (in the readme) for the client config builder since it's becoming so important.

It would be cool to create good reference docs (as readmes) for wasm-js and plugin-js, and really every package.

Ideally we'd have the package creators also write up documentation, or at least a basic outline of it (with functional details), and have someone else (e.g. me, you, any volunteer) fix it up to spec.

…eadme

…ption of polywrap client in readme to core client; replaced "queries" word with "invocations"

…tion to client config builder README.md

… elsewhere for interface types

…t markers

…-builder-js

…e pulled into readme by doc-snippets

…eadme

pileks · 2023-01-06T14:19:28Z

Should we consider merging this and opening documentation issues for the remaining packages?

…eadme # Conflicts: # packages/js/client-config-builder/src/bundles/getDefaultConfig.ts # packages/js/core-client/src/PolywrapCoreClient.ts # packages/js/core/src/interfaces/uri-resolver.ts # packages/js/test-env/src/index.ts

krisbitney · 2023-01-10T13:49:32Z

Should we consider merging this and opening documentation issues for the remaining packages?

Yeah, I think that's a good idea. It will prevent later work fixing merge conflicts.

…eadme # Conflicts: # packages/js/client-config-builder/package.json # packages/js/client/package.json # packages/js/core-client/package.json

dOrgJelli

This all looks great! Just a few improvements I think are needed before merging.

dOrgJelli · 2023-01-12T02:02:51Z

+
+### Constructor
+```ts
+   * Instantiate a ClientConfigBuilder


This section doesn't appear to be rendering correctly, it's missing the starting /*

dOrgJelli · 2023-01-12T02:09:57Z

 ```

-## Methods
+## Types


Above this line, in the introduction section, I think it could use some work. A few thoughts:

I don't think "DSL" is properly used here.

Ideally all code snippets should be from-source so that they remain up-to-date, so this one that's not a snippet worries me as it can become out-of-date.

I think should start with introducing the main concepts a user must understand to get started. We can introduce the intended usage pattern of init -> configure -> build, and we can point to sub-sections for each. "Initialization" shows the constructor signature. "Configure" shows the different "add..." methods. etc

Lastly, in this intro section, having an e2e example snippet (taken from a buildable source file) would be great to have.

So in summary:

Brief Description

Intro Concepts (linking to reference sections further down)

E2E Example Snippet

Apologies if this is overly verbose :P

i reworked this intro and used snippets as suggested in your other comment

dOrgJelli · 2023-01-12T02:15:02Z

+}
+```
+
+## ClientConfigBuilder


Can we group the ClientConfigBuilder and ClientConfig sections underneath a Reference section?

dOrgJelli · 2023-01-12T02:20:58Z

+        "inline": false
+      },
+      {
+        "pattern": "/* $: {SNIPPET_NAME} */",


Why do we have {SNIPPET_NAME} here, but not in the above start token?

Oh nvm, I think I figured it out, it's because this one is "inline" so it can't know where to extract the snippet name from like it can in the pattern above. Is that correct?

right, that's correct. The {SNIPPET_NAME} helps define a regex pattern.

dOrgJelli · 2023-01-12T02:24:43Z

+### Invoke a wrapper
+
+```ts
+await client.invoke({


We should convert this to a code snippet so we make sure it runs. Maybe extract it from the tests folder or something.

Also it seems like we have this problem with the other READMEs I'm seeing here, all the example code snippets aren't being extracted from source. Might I suggest us adding an examples/ folder next to the src/ folder where we keep these examples? They would be built so we can make sure they are correct, but their artifacts will not make it into the build/ folder so they do not get published with the built src/ files.

Another option is to create a src/__snippets__ folder, that lives alongside the src/__tests__ folder.

I added examples/ folders and set up snippets in client, core-client, client-config-builder, and test-env.

…eadme

…dded "build:readme" to build scripts and added "build:fast" options; added "Reference" section headers to readmes

…tract snippets from examples

krisbitney · 2023-01-14T17:54:06Z

@dOrgJelli Your suggestions were really good. Thanks for looking at this!

I think I addressed all of your comments: I updated the client config builder readme. All code examples now use snippets that build with yarn build. We can use yarn build:fast to just build the source code.

I think it's in much better shape and ready for a new review!

dOrgJelli

👏 🔥 ✔️

…eadme # Conflicts: # packages/js/client-config-builder/src/ClientConfigBuilder.ts # packages/js/core/src/uri-resolution/UriResolutionContext.ts # packages/js/test-env/src/index.ts # yarn.lock

updated readme for client and core-client

a095840

krisbitney requested review from dOrgJelli, namesty and nerfZael as code owners December 20, 2022 15:59

krisbitney marked this pull request as draft December 20, 2022 15:59

pileks previously approved these changes Dec 20, 2022

View reviewed changes

cbrzn reviewed Dec 21, 2022

View reviewed changes

Comment thread packages/js/core-client/README.md Outdated

Comment thread packages/js/client/README.md Outdated

cbrzn reviewed Dec 21, 2022

View reviewed changes

Comment thread packages/js/client/README.md Outdated

krisbitney added 2 commits December 22, 2022 14:21

Merge remote-tracking branch 'origin/origin-dev' into update-client-r…

021587d

…eadme

removed "rinkeby" from client readme invocation example; moved descri…

611951e

…ption of polywrap client in readme to core client; replaced "queries" word with "invocations"

krisbitney dismissed pileks’s stale review via 611951e December 22, 2022 09:31

krisbitney added 14 commits December 27, 2022 18:57

removed "rinkeby" from core client readme invocation example

195f3b3

added TSDoc comments to client config builder and added reference sec…

e4c1226

…tion to client config builder README.md

added client configs to client, core client, and config builder readmes

3eec7e9

adjusted client config comments to more closely match formatting used…

4a81221

… elsewhere for interface types

added TSDoc comments for CoreClient.ts

18f98be

replaced @polywrap/client-js readme reference content with doc snippe…

618afcd

…t markers

using doc-snippets package to build readme in @polywrap/client-js

8e4ad88

using doc-snippets package to build readme in @polywrap/core-client-js

7217c66

using doc-snippets package to build readme in @polywrap/client-config…

baca401

…-builder-js

using doc-snippets package to build readme in @polywrap/core-js

9a010e1

Added TSDoc comments throughout @polywrap/core-js and ensured they ar…

d7a603f

…e pulled into readme by doc-snippets

lint

cefc757

Merge remote-tracking branch 'origin/origin-dev' into update-client-r…

51dcee8

…eadme

updated doc-snippets version to 0.4.0-pre.4 to use inline snippets

8e03658

krisbitney added 3 commits January 10, 2023 18:17

added doc comments and readme updates to test-env-js

e7a92b4

merge conflict fix

6cae2eb

fixed typo made when resolving merge conflicts

8df2fc9

pileks previously approved these changes Jan 10, 2023

View reviewed changes

krisbitney marked this pull request as ready for review January 10, 2023 17:52

krisbitney requested a review from cbrzn January 10, 2023 17:53

Merge remote-tracking branch 'origin/origin-dev' into update-client-r…

d54e0dd

…eadme # Conflicts: # packages/js/client-config-builder/package.json # packages/js/client/package.json # packages/js/core-client/package.json

krisbitney dismissed pileks’s stale review via d54e0dd January 11, 2023 10:22

pileks previously approved these changes Jan 11, 2023

View reviewed changes

dOrgJelli requested changes Jan 12, 2023

View reviewed changes

krisbitney added 2 commits January 14, 2023 18:28

Merge remote-tracking branch 'origin/origin-dev' into update-client-r…

fdfd5b8

…eadme

fixed typo in client config builder constructor snippet extraction; a…

1b8ca78

…dded "build:readme" to build scripts and added "build:fast" options; added "Reference" section headers to readmes

krisbitney dismissed pileks’s stale review via 1b8ca78 January 14, 2023 13:19

krisbitney added 5 commits January 14, 2023 20:42

reworked intro in client config builder readme using buildable snippets

bacd790

reworked doc-snippets config for client config builder to properly ex…

a748ca8

…tract snippets from examples

added snippets to client readme intro

32350d8

added snippets to core client readme intro

f23c36e

added snippets to test-env-js readme intro

a11986c

krisbitney requested review from dOrgJelli and pileks January 14, 2023 17:55

dOrgJelli previously approved these changes Jan 17, 2023

View reviewed changes

Merge branch 'origin-dev' into update-client-readme

9f8abb7

dOrgJelli dismissed their stale review via 9f8abb7 January 18, 2023 17:48

krisbitney added 2 commits January 19, 2023 15:01

Merge remote-tracking branch 'origin/origin-dev' into update-client-r…

d310352

…eadme # Conflicts: # packages/js/client-config-builder/src/ClientConfigBuilder.ts # packages/js/core/src/uri-resolution/UriResolutionContext.ts # packages/js/test-env/src/index.ts # yarn.lock

readme updates

66ceb48

dOrgJelli merged commit 52fec66 into origin-dev Jan 20, 2023

dOrgJelli deleted the update-client-readme branch April 10, 2023 16:59

Conversation

krisbitney commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pileks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krisbitney Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pileks commented Jan 6, 2023

Uh oh!

krisbitney commented Jan 10, 2023

Uh oh!

dOrgJelli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krisbitney commented Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dOrgJelli left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

krisbitney commented Dec 20, 2022 •

edited

Loading

krisbitney Dec 20, 2022 •

edited

Loading

krisbitney commented Jan 14, 2023 •

edited

Loading