Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(patterns): start with simple example #2442

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dckc
Copy link
Contributor

@dckc dckc commented Sep 3, 2024

Description / Documentation Considerations

Start the README with a simple example (taken from a test). Make the PatternMatchers interface that defines M's methods easier to find.

  • Demote some material to the JSDoc of relevant types.
  • add various links
  • Demote some stuff to CONTRIBUTING. (Maybe it's redundant and should go away.)

Security / Scaling / Testing / Compatibility / Upgrade Considerations

n/a

@dckc dckc requested review from warner and erights September 3, 2024 04:18

TODO: Include a diagram visually demonstrating the following.
Builds on {@link @endo/pass-style!} as described in [`kindOf` and `passStyleOf` levels of abstraction](./docs/marshal-vs-patterns-level.md) to define higher level data types as individual refinements of Passable CopyTagged records (PassStyle "tagged"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't suppose this sort of link markup works in the https://www.npmjs.com/package/@endo/patterns context.

We should have a link from there to https://endojs.github.io/endo/modules/_endo_patterns.html or at least to https://endojs.github.io/, I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

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

What language is @link from? It certainly doesn't render in GitHub-Flavored-Markup (GFM). I'm assuming our docs site?

Yeah, I'd suggest having the first @link or two in the file include a parenthetical non-obscured URL (so https://endojs.github.io/ or https://docs.agoric.com/ or whatever, rather than ["the docs"](https://endojs.github.io)) to a site where the rendered form of this file is served, hopefully one or two obvious clicks away from the other linked URLs.

My usual "how does this work" search sequence starts with the import statement, getting me to a package name, and then I jump to the source code that defines it (which, for Endo stuff, is on my laptop, but the GitHub repo is the next stop after that). I expect the source code to have something readable, else I expect the GitHub markup renderer to provide something readable, so when there's a different docs site that has the actually-usable version, I love it when the README has a "rendered docs available ->HERE<-" pointer.

Copy link
Contributor

Choose a reason for hiding this comment

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

const expected = M.splitRecord(
{ foo: M.number() },
{ bar: M.string(), baz: M.number() },
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be correct to have this say:

  { foo: M.number() }, // required properties
  { bar: M.string(), baz: M.number() }, // optional

so a newbie to M like me gets a hint as to why there are two arguments?

* key distributed equality semantics.)
*
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if the rank/key order details would fit better in CONTRIBUTING.md along with the other details extracted from README. I don't think I'd look for details like that in a types.js file (although tbh I'm not sure "CONTRIBUTING.md" is where I'd look either, maybe something like IMPLEMENTATION-DETAILS.md).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it in CONTRIBUTING for a bit, but rankCompare and keyCompare are part of the api, not an implementation detail.

The material could perhaps move to those functions, but I suspect the editorial work would exceed my timebox for this.

Copy link
Contributor

@warner warner left a comment

Choose a reason for hiding this comment

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

Looks like a good improvement! Note I have not verified that the moved text did not change, just that the additions to README are helpful to me.

- CopyBag -- a collection of entries associating a unique distinguishable Key with a positive integer count (see [Multiset](https://en.wikipedia.org/wiki/Multiset)).
- CopyMap -- a collection of entries associating a unique distinguishable Key with a Passable
- Matcher -- a predicate characterizing a subset of Passables, such as "strings" or "8-bit unsigned integer numbers" or "CopyArrays of Remotables"
The main export from the package is an {@link M} namespace object, for making a variety of Matchers (hence "M"). See {@link PatternMatchers} for relevant methods.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll guess that this {@link M} form is better for the generated documentation, which is great. But when viewed directly on github it does not render as a link or anything useful. Especially a README.md file should be pleasantly readable directly on github.

Can we get both somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can write some links one way and some the other. I can sort of imagine demoting the {@link X} things so that they don't get in the way in non-typedoc contexts.

@dckc dckc marked this pull request as draft September 6, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants