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

[Documentation] Markdown files review and approval proposal. #4520

Closed
Ressetkk opened this issue Dec 2, 2021 · 3 comments
Closed

[Documentation] Markdown files review and approval proposal. #4520

Ressetkk opened this issue Dec 2, 2021 · 3 comments
Assignees
Labels
area/ci Issues or PRs related to CI related topics area/documentation Issues or PRs related to documentation decision Related to all issues that need a decision

Comments

@Ressetkk
Copy link
Contributor

Ressetkk commented Dec 2, 2021

As a part of the improvements for development and workflow around Kyma I have a small proposal for the documentation approval and review flow. Right now we are using Prow without the full potential of approve and review flow because by definition all Markdown files are required to be approved by the technical writers group. This assumption let alone is really good, however it blocks the ability to introduce the approve plugin for Prow and blocks us from revoking the organisation members write access to the repository. By default approve plugin does not allow assigning technical writers group to review only specific files determined by a wildcard and there is still ongoing issue kubernetes/test-infra#7690. The new implementation would require complete rewrite to make it work, and there is no KEP defined for that functionality yet. Instead of waiting for upstream we should take some action to un-block the ability to introduce the approve plugin and comment flow that Prow utilises.

Here are 2 of my proposals:

  1. Technical writers are not required for the markdown files to be reviewed inside the various repos and be a responsibility for the code owners of the given directory. Of course the author can always ask the technical rites group for a review, but it won't be necessary for the PR to be merged.
  2. Introduce Prow plugin that checks wether the Markdown files are changed and adds a label needs-tws-review to the PR. The label would work as a blocking label. Given that the members will have no write access to the repo they won't be able to remove the label at all. Once the PR is approved by the member of the technical writers group the PR will be un-blocked and Tide will continue with a merge of the PR. Once the changes to the markdown files are made again it will re-add the label and comment that changes to the markdown have been made.

All options try to tackle problem which blocks us from introducing approve plugin forProw on the kyma-project and kyma-incubator organisations.

@Ressetkk Ressetkk added area/documentation Issues or PRs related to documentation area/ci Issues or PRs related to CI related topics decision Related to all issues that need a decision labels Dec 2, 2021
@dekiel
Copy link
Contributor

dekiel commented Dec 2, 2021

Let's try second option.

@Ressetkk
Copy link
Contributor Author

There is opened Pull Request regarding gradual approvals so the plug-in probably won't be needed at all. kubernetes/test-infra#21398

@Ressetkk Ressetkk self-assigned this Dec 14, 2021
@Ressetkk
Copy link
Contributor Author

Plugin is working, codebase is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Issues or PRs related to CI related topics area/documentation Issues or PRs related to documentation decision Related to all issues that need a decision
Projects
None yet
Development

No branches or pull requests

2 participants