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

[EngAut] generate prdoc ui PoC #244

Closed
mordamax opened this issue Oct 14, 2023 · 8 comments
Closed

[EngAut] generate prdoc ui PoC #244

mordamax opened this issue Oct 14, 2023 · 8 comments
Assignees

Comments

@mordamax
Copy link
Contributor

mordamax commented Oct 14, 2023

Related discussion: https://forum.parity.io/t/pr-documentation-prdoc/2065/21

In the future the prdocs will be a part of CI which will block PR unless either 1) the prdoc file for PR is provided 2) PR has the label R0-silent

But for this to happen - we need to improve the devX by providing a simpler way to provide documentation for PR

The idea was - if the CI detected that prdoc was missing - send a link to some UI, which would provide an easier way to tell what was the change in PR and to whom this change relates. UI could enhance devX by providing the fields with an explanation and examples of how to document PR and what the audiences are etc

Obstacles: command-bot does not provide UI for now (tbd #128)

@mordamax mordamax changed the title generage prdocs generate prdocs Oct 14, 2023
@mutantcornholio mutantcornholio self-assigned this Oct 18, 2023
@mutantcornholio
Copy link
Contributor

I'm thinking that this could be a static GitHub-pages UI, and even a part of prdoc repo itself.
I'll try to make a PoC for it

https://forum.parity.io/t/pr-documentation-prdoc/2065/25?u=yuri_volkov

@chevdor
Copy link

chevdor commented Oct 18, 2023

Beware, in the first version, we had ONE schema.
I am working on a new version where the schema will be configurable, per repo, and stored in the project's repo (ie not in the prdoc repo).

So prdoc will no longer have information about a specific schema in its repo (a schema will remain but will be used for testing mainly).

@mordamax mordamax changed the title generate prdocs [EngAut] generate prdoc ui PoC Oct 19, 2023
@mutantcornholio
Copy link
Contributor

mutantcornholio commented Nov 1, 2023

And just when I was ready to test it, I found out that GitHub OAuth API doesn't support CORS: isaacs/github#330.
After some research it seems that it's actually impossible to create a clientside-only app, that would use GitHub API.
Seems that we would need a server just to keep client_secret and do key exchange.

UPD: @rzadp have suggested using "add file" links like this, which solves the problem

@mutantcornholio
Copy link
Contributor

mutantcornholio commented Nov 8, 2023

@chevdor Hi! Here's an option how that could look: https://mutantcornholio.github.io/prdoc/?pull=16&org=paritytech&repo=prdoc&branch=wk-231019-completions
If you click "Submit", it'll take you to a link like this, which would add a file to your repository.

I've pushed the source code here, however, this is a PoC, so it's not something that's remotely ready for merge.
Some of the questions that are yet to be resolved / work to be done:

  • prdoc utility supports templating, while this tool currently just does yaml.stringify, which produces less readable results
  • I had to do some changes in the schema, in order to adapt it to uniforms. The changes kinda make sense, but I haven't tested prdoc utility with them yet
  • There's the question of generating of links for the ui, and I would probably put it to check-prdoc.yml. Maybe make it reusable.
  • The question of a character limit in url
  • Supporting schemas that are defined in target repositories
  • Validation of schemas both according to prdoc utility, as well as ui

@chevdor
Copy link

chevdor commented Nov 8, 2023

Woo that looks like a neat option !
You made a copy of a the schema, so I don't see the diff easily since upstream has changed.
Could you update to the current schema ? It is the one added to the SDK repo in paritytech/polkadot-sdk#1946

@mutantcornholio
Copy link
Contributor

https://github.com/paritytech/prdoc/compare/yuri/prdoc-gen-poc?expand=1#diff-013e70c9bc6c6ea9370d573976ee48fd49cf49142968700bdb9b37a48d105c27
Here are the changes for the schema in prdoc, I'll try to adapt polkadot's new one a bit later

@chevdor
Copy link

chevdor commented Nov 8, 2023

You have changes related to required, I don't think those will be problematic, that will be defined my the schema and needs to be updated accordingly.

The audience as enum is an issue. I did start with an enum as well but I switched to const on purpose since we need the documentation for the audience and this is a rather critical point.

Are there resons why you removed:

  • version
  • $schema
    ?

@mutantcornholio
Copy link
Contributor

mutantcornholio commented Nov 9, 2023

Most of the reasons were cutting corners, to get PoC working.
After googling for a bit, I already managed to fix version and $schema, but audience may be not so simple. If we want to go with the idea, I can experiment a bit more on this.

UPD: I kinda was able to do that and retain the description in the schema, using options and UI label:

Code example
  "$defs": {
    "audience": {
      "description": "You may pick one or more audiences and address those users with appropriate documentation, information and warning related to the PR.",
      "type": "string",
      "options": [
        {
          "value": "Node Dev",
          "label": "Node Dev",
          "title": "Node Dev",
          "description": "Those who build around the client side code. Alternative client builders, SMOLDOT, those who consume RPCs. These are people who are oblivious to the runtime changes. They only care about the meta-protocol, not the protocol itself."
        },
        {
          "value": "Runtime Dev",
          "label": "Runtime Dev",
          "title": "Runtime Dev",
          "description": "All of those who rely on the runtime. A parachain team that is using a pallet. A DApp that is using a pallet. These are people who care about the protocol (WASM), not the meta-protocol (client)."
        },
        {
          "value": "Node Operator",
          "label": "Node Operator",
          "title": "Node Operator",
          "description": "Those who don't write any code and only run code."
        },
        {
          "value": "Runtime User",
          "label": "Runtime User",
          "title": "Runtime User",
          "description": "Token holders who don't run anything but are involved in the network."
        }
      ]
    },

However, the problem is, the description is not displayed there.
I think, I can probably create a custom component for cases like this.
Screen Shot 2023-11-09 at 11 55 03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants