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 manifest 0.1.0 #1735

Merged
merged 38 commits into from
Jun 19, 2023
Merged

Docs manifest 0.1.0 #1735

merged 38 commits into from
Jun 19, 2023

Conversation

pileks
Copy link
Contributor

@pileks pileks commented May 9, 2023

This PR introduces the Documentation Manifest, an extension manifest for the WASM/Interface project manifests.

An example documentation manifest:

format: 0.1.0
title: A test wrap docs
logo: ./docs/logo2.png
pages: # the keys of this object are used as URI slugs for the readmes
  home:
    title: Home page
    path: ./docs/pages/first.md
  second-page:
    title: Second page
    path: ./docs/pages/second.md
homePage: second-page # we can specify any page to be our landing page
examples: # the keys of this object are used as URI slugs for the examples
  foo:
    title: Foo
    steps:
      - uri: $$WRAP_URI
        method: sampleMethod
        args:
          arg: Foo 1
        description: This is a foo 1 on URI $$WRAP_URI
      - uri: $$WRAP_URI
        method: sampleMethod
        args:
          arg: $0.value.result
        description: This is a foo 2 description $0.value.result value injection!

I have decided against merging workflow jobs and doc examples at this time for the following reasons:

  • Workflows don't have the concept of a "primary job" in the same way deploy manifests do. Adding this just to satisfy the needs of the docs manifest seems counter-intuitive
    • Now that I say it out loud, adding a jobName field to the workflow might work. Thoughts? (Still doesn't answer other points)
  • There is no real use for step.description in Workflows themselves, it would be a Docs addition to a manifest that doesn't belong to Docs
  • Easier overall to work on them separately first - we can decide to merge them later or even diverge if we see it go that way

If polywrap.docs.yaml is present and mentioned in exts of the Project Manifest, polywrap build outputs all files related to documentation inside a docs directory within the build output dir.

This directory can then be consumed by wrap-docs, as is described in the following PR:
polywrap/wrap-docs#8

@pileks pileks mentioned this pull request May 10, 2023
4 tasks
@pileks pileks marked this pull request as ready for review May 10, 2023 12:03
@pileks pileks self-assigned this May 10, 2023
@krisbitney
Copy link
Contributor

I think the way you handled the docs examples is great. The execution of the examples can use same backend as polywrap.test.yaml workflows, right? If not, we should generalize the back end so it can run both polywrap.test.yaml examples and polywrap.docs.yaml workflows. Essentially, the docs manifest lets users define a workflow with metadata.

An alternative would be to only put the metadata in the docs manifest and ask users to provide a link to their workflow file, but I think the UX is better the way you've done it.

krisbitney
krisbitney previously approved these changes May 11, 2023
@krisbitney
Copy link
Contributor

krisbitney commented May 11, 2023

One thought - I think users should also be able to reference assets stored in the resources directory, if that's not already possible. That way users never feel a need to have duplicate copies of files. Or maybe the docs directory should be stored in the resources folder? I'm not sure.

That kind of feature could come in a future PR

@pileks
Copy link
Contributor Author

pileks commented May 15, 2023

I think the way you handled the docs examples is great. The execution of the examples can use same backend as polywrap.test.yaml workflows, right? If not, we should generalize the back end so it can run both polywrap.test.yaml examples and polywrap.docs.yaml workflows. Essentially, the docs manifest lets users define a workflow with metadata.

An alternative would be to only put the metadata in the docs manifest and ask users to provide a link to their workflow file, but I think the UX is better the way you've done it.

This was actually the original idea!
I've detailed out the reasons against going this path in the description, and it's something that I'll be keeping in mind for the future!

One thought - I think users should also be able to reference assets stored in the resources directory, if that's not already possible. That way users never feel a need to have duplicate copies of files. Or maybe the docs directory should be stored in the resources folder? I'm not sure.

That kind of feature could come in a future PR

This is not a bad idea at all. I'll see that we have something like this added.

Copy link
Contributor

@dOrgJelli dOrgJelli left a comment

Choose a reason for hiding this comment

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

I think we should remove the "pages" entry in the manifest, to help simplify this new feature. Let's only keep the features we need for initial MVP of wrapscan.

To me this is:

  • description
  • logo
  • repository
  • website
  • examples

@pileks
Copy link
Contributor Author

pileks commented Jun 17, 2023

I think we should remove the "pages" entry in the manifest, to help simplify this new feature. Let's only keep the features we need for initial MVP of wrapscan.

To me this is:

  • description
  • logo
  • repository
  • website
  • examples

I believe the correct fields for the Wrapscan MVP are:

  • Description
    • this one is obvious - each Wrap should have a description rendered in the list and above its readme page
    • We would be duplicating this information within Wrapscan's homepage for starters, and later moving it into some form of incremental cache
  • Logo
    • this one is necessary too, same reason as description, same caveats
  • Readme
    • a path to the readme file. I'd keep this so that we can later add multiple readme pages easily, and docs manifest migrations will be easier to do if we immediately have paths instead of writing to a default readme path (we're not renaming files, we're just placing them in the appropriate place ahead-of-time, and the code for this is already completed).
  • Repository URL - the wrap info widget renders this
  • Website URL - the wrap info widget renders this

We don't need the Examples field actually, as it's not part of the MVP. Nevertheless, I'd keep it as the sturcture seems good enough, and we'll be able to add examples much quicker if we just keep it.

@pileks pileks requested review from dOrgJelli and krisbitney June 17, 2023 14:14
"uri": {
"description": "URI of the wrap being invoked.",
"type": "string",
"tsType": "any"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Don't we want this to be typed a "string"?

dOrgJelli
dOrgJelli previously approved these changes Jun 19, 2023
@dOrgJelli dOrgJelli merged commit e6d5f55 into origin-dev Jun 19, 2023
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.

3 participants