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

Improve prdoc Generation #46

Open
AndreiEres opened this issue Sep 24, 2024 · 10 comments
Open

Improve prdoc Generation #46

AndreiEres opened this issue Sep 24, 2024 · 10 comments
Assignees

Comments

@AndreiEres
Copy link

The current functionality of prdoc generate <NUMBER> is essentially copying pr_template.prdoc to pr_<NUMBER>.prdoc. Assuming that a prdoc can be generated after a PR is almost ready, I would like to propose some enhancements to automate this process further:

  1. Automatic PR Number: Instead of requiring a PR number, the command could be simplified to prdoc generate and automatically query the current PR number.
  2. Output Path Display: After generating the file, the path to the generated file could be displayed in the output so it can be opened immediately from the terminal.
  3. Pre-filled Audiences: Including all possible audiences in the template, e.g., - audience: [Node Dev, Runtime Dev, ...], with a comment saying "Remove audiences you don’t need" could help in remembering the available audiences.
  4. Auto-fill Title: The title could be automatically filled from Github PR.
  5. Auto-fill Description: The full Github PR description could be pasted into the description field, allowing the user to remove obsolete parts and edit it directly in prdoc.
  6. Auto-fill Semver Checks: We could use our semver checks to automatically fill in details about which crates were changed and the severity of those changes.

I believe these improvements can make the work with prdoc smoother and more efficient. I would appreciate any feedback or suggestions on these proposed changes.

@mutantcornholio
Copy link
Contributor

I don't feel that Auto-fill Semver Checks is currently feasible, as it's the doing of https://github.com/paritytech/parity-publish, which is dependent on prdoc itself.

Not a huge fan of Pre-filled Audiences, but sounds good for an optional flag.

Automatic PR Number, Auto-fill Title and Auto-fill Description would require GitHub token, of course, and also, finding the PR for the current fork+branch, which is a bit more complex than it sounds, but all doable, at least for a normal case, where there's only one PR from that branch.

@s0me0ne-unkn0wn
Copy link

I do agree that all the proposed changes would be helpful. Pre-filled audiences are better as an option indeed, as well as the auto-filled description (I wouldn't like something like paritytech/polkadot#6161 to be copied to a prdoc by default 🫠).

@mordamax
Copy link

may be better to use /cmd prdoc for that? not sure if the prdoc cli itself should include that functionality

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/prdoc.md

/cmd prdoc --help

@sandreim
Copy link

I find number 6 particularly useful if your PR changes many crates. Generally I would love to avoid doing manual grunt work on the PRDocs.

@alindima
Copy link

may be better to use /cmd prdoc for that? not sure if the prdoc cli itself should include that functionality

https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/prdoc.md

/cmd prdoc --help

Yes, I think this would be best!
I am very supportive of the suggested improvements

@mordamax
Copy link

currently /cmd prdoc already fills in the description from the description of PR and pushes the file, but could be improved from there.
Feel free to contribute:

@AndreiEres
Copy link
Author

AndreiEres commented Sep 27, 2024

may be better to use /cmd prdoc for that? not sure if the prdoc cli itself should include that functionality

I would prefer not to use it for autofilling the title and description. I believe a PR doc should be written within five minutes after we have most of the information about our PR. We write in the PR /cmd, wait for the PR doc to be created, pull it to the local branch, edit it, push it, wait for the semver checks, what next? This process doesn’t seem like automation.

@mordamax
Copy link

@AndreiEres no you fill it in with arguments from the command, i guess in most cases there's no need to edit it again on local and push. The description is being filled from the PR description, title and audience you provide via args

@ggwpez
Copy link
Member

ggwpez commented Sep 27, 2024

could help in remembering the available audiences

There is a JSON schema that you can install that gives your IDE hints and auto-complete.

Auto-fill Title
Auto-fill Description

This is done by the prdoc command, or you can run the script locally, works the same.

Generally we should try to make the prdoc CLI more ergonomic. It is now a Parity fork, and we can deviate from its initial design goal of being un-opinionated and general. Maybe integrating the logic of that python script and using GH cli or something to resolve the current MR number gh pr view --json number --jq '.number'.

Pre-filled Audiences

We had this in the past, and noone edited it. Most PRdocs merged with wrong audience.

Auto-fill Semver Checks

This is difficult since it requires specific compiler versions and then compiles each crate twice. Not impossible to reproduce locally, but probably not easy.
The thing is also that devs are still supposed to increase the bump level when they seem fit. The check can never spot cases that have semantic changes in APIs.

@mordamax
Copy link

after some DM conversations, the final plan looks like this:

  • implement "generate" function as described in PR (or as close as possible to this)
  • remove generate-prdoc.py from .github/scripts
  • /cmd prdoc will be switched from py to native pr doc generate --args

this way we will have the same generate capabilities working locally and/or from PR

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

No branches or pull requests

7 participants