docs: Split, update, improve the contributing guide for reviewers and committers #27085

qmonnet · 2023-07-26T12:25:47Z

docs: Request sentence case for headings in documentation
docs: Break down contributing guide
docs: Improve source formatting on review process
docs: Consolidate review process, update periodic duties description
docs: Add guidance for cilium/docs-structure reviews

Easier to review per commit.

Cc @aanm @margamanterola (Contributing guide & weekly duties update)
Cc @learnitall @lambdanis (docs-structure review guide)

learnitall

Thank you for this @qmonnet ❤️! This looks great! There are a couple of places that I think need some more background information to help users, but other than that I just have a bunch of nits on sentence structure, word choice, and voice. Also, IMO I think we should try to avoid using parentheses when possible, and there are a lot of parentheses used in these changes, so I made some suggestions on rephrasing things to remove them.

Documentation/contributing/development/contributing_guide.rst

Documentation/contributing/development/reviewers_committers/duties.rst

Documentation/contributing/development/reviewers_committers/review_docs.rst

margamanterola

LGTM with regards to rotations and structure. I did a quick read of the whole text and didn't find any issues to highlight.

learnitall

Awesome looks great! Thank you!

Documentation/contributing/development/reviewers_committers/duties.rst

zacharysarah

@qmonnet Thank you so much for this! I have some wording suggestions and another plea to use sentence casing for headings. 🙏🏻

Documentation/contributing/development/contributing_guide.rst

Documentation/contributing/development/reviewers_committers/duties.rst

Documentation/contributing/development/reviewers_committers/review_process.rst

qmonnet · 2023-07-27T10:17:36Z

Silly me, I forgot to Cc @joestringer, who of course has been taking good care of documentation as well 🙀. Apologies, Joe!

joestringer

LGTM, couple of minor nits and one potential content change that should maybe be handled separately to avoid stacking a subtle process change into this PR which is otherwise general improvements.

Documentation/contributing/development/reviewers_committers/review_process.rst

Documentation/contributing/development/reviewers_committers/duties.rst

Documentation/contributing/development/reviewers_committers/review_docs.rst

zacharysarah

Thanks for the updates, @qmonnet!

joestringer · 2023-07-31T19:28:14Z

/test

As discussed by some documentation reviewers, update the style guide to instruct contributors to use sentence case rather than title case on headings in documentation. We don't update the whole documentation, which has never been consistent in that regard. However, we do update the guidance for writing documentation itself, as these pages should set an example for the rest of the docs, and should follow the guidance they set themselves. Suggested-by: Sarah Corleissen <sarah.corleissen@isovalent.com> Signed-off-by: Quentin Monnet <quentin@isovalent.com>

The Contributing guide is long. Big. Huge. Let's make it easier to read by splitting it into three parts: - Guidance for community contributors (remaining on the same page) - Guidance for reviewers and committers, for the review process - Guidance for reviewers and committers, for weekly duties Reference the new pages from the existing one. Apart for title formatting and new reference target, the contents in the new page are unchanged at this stage. Signed-off-by: Quentin Monnet <quentin@isovalent.com>

Minor formatting improvements: avoiding inline link URLs, and re-wrapping the paragraphs, to improve readability of the source for the document. Signed-off-by: Quentin Monnet <quentin@isovalent.com>

Before this commit, the page on weekly duties would contain: - A description of the PR review process duty, which describes much of the PR review process, and should really be in the page dedicated to this process - An outdated description of the other rotations (backporter, janitor). Let's try to improve and update this. First, we move everything related to the review process to the dedicated page. Doing so, we update the contents to remove any reference to a weekly duty, and to keep only the practical aspects of the process. Then, we rewrite the description of the weekly duties: we currently have release manager, backporter, triager, and CI health manager. The description for these roles is rather succinct, because I don't think they are publicly documented anywhere at the moment, but we can improve this in the future. We're also missing a way to identify who are the assignees at this time. As Joe observed, some of these duties do not rotate on a weekly basis; so we also rename the page into "Periodic duties", and update the description accordingly. Signed-off-by: Quentin Monnet <quentin@isovalent.com>

qmonnet · 2023-08-01T08:30:20Z

I updated the last paragraph about ready-to-merge docs PRs, in the docs-structure document, now that we can and should run /test for all docs PRs.

/test

Add a new document, among the guidance for reviewers/committers, to detail the particularities and expectations for reviewing Pull Requests as a member of the cilium/docs-structure team. Signed-off-by: Quentin Monnet <quentin@isovalent.com>

qmonnet · 2023-08-01T08:32:15Z

/test

qmonnet added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact. labels Jul 26, 2023

qmonnet requested a review from zacharysarah July 26, 2023 12:25

qmonnet requested review from a team as code owners July 26, 2023 12:25

qmonnet requested a review from bimmlerd July 26, 2023 12:25

qmonnet force-pushed the pr/docs-contrib-split branch from 4b2161d to bafdf45 Compare July 26, 2023 12:37

learnitall requested changes Jul 26, 2023

View reviewed changes

margamanterola approved these changes Jul 26, 2023

View reviewed changes

qmonnet force-pushed the pr/docs-contrib-split branch from bafdf45 to d3dc717 Compare July 26, 2023 16:38

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jul 26, 2023

qmonnet force-pushed the pr/docs-contrib-split branch from 352d5d5 to 16d9a7d Compare July 26, 2023 16:51

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jul 26, 2023

cilium deleted a comment from maintainer-s-little-helper bot Jul 26, 2023

qmonnet requested a review from learnitall July 26, 2023 16:51

qmonnet force-pushed the pr/docs-contrib-split branch from 16d9a7d to f5083d4 Compare July 26, 2023 16:58

learnitall approved these changes Jul 26, 2023

View reviewed changes

Documentation/contributing/development/reviewers_committers/duties.rst Outdated Show resolved Hide resolved

qmonnet mentioned this pull request Jul 26, 2023

docs: Document Cilium's documentation build framework #27098

Closed

qmonnet force-pushed the pr/docs-contrib-split branch from f5083d4 to b4bac5b Compare July 26, 2023 17:07

zacharysarah requested changes Jul 26, 2023

View reviewed changes

qmonnet force-pushed the pr/docs-contrib-split branch from b4bac5b to ff7bf63 Compare July 27, 2023 09:50

joestringer approved these changes Jul 27, 2023

View reviewed changes

qmonnet force-pushed the pr/docs-contrib-split branch from ff7bf63 to 261155b Compare July 28, 2023 01:05

qmonnet requested a review from zacharysarah July 28, 2023 01:06

aanm self-requested a review July 28, 2023 15:53

zacharysarah approved these changes Jul 28, 2023

View reviewed changes

qmonnet added 2 commits August 1, 2023 09:21

docs: Improve source formatting on review process

5907c23

Minor formatting improvements: avoiding inline link URLs, and re-wrapping the paragraphs, to improve readability of the source for the document. Signed-off-by: Quentin Monnet <quentin@isovalent.com>

qmonnet force-pushed the pr/docs-contrib-split branch from 261155b to 5b4c2a1 Compare August 1, 2023 08:30

qmonnet force-pushed the pr/docs-contrib-split branch from 5b4c2a1 to 534ca1b Compare August 1, 2023 08:32

aanm approved these changes Aug 3, 2023

View reviewed changes

aanm merged commit 9f4b4df into cilium:main Aug 3, 2023
55 checks passed

qmonnet deleted the pr/docs-contrib-split branch August 3, 2023 14:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Split, update, improve the contributing guide for reviewers and committers #27085

docs: Split, update, improve the contributing guide for reviewers and committers #27085

qmonnet commented Jul 26, 2023 •

edited

learnitall left a comment

margamanterola left a comment

learnitall left a comment

zacharysarah left a comment

qmonnet commented Jul 27, 2023

joestringer left a comment

zacharysarah left a comment

joestringer commented Jul 31, 2023

qmonnet commented Aug 1, 2023 •

edited

qmonnet commented Aug 1, 2023

docs: Split, update, improve the contributing guide for reviewers and committers #27085

docs: Split, update, improve the contributing guide for reviewers and committers #27085

Conversation

qmonnet commented Jul 26, 2023 • edited

learnitall left a comment

Choose a reason for hiding this comment

margamanterola left a comment

Choose a reason for hiding this comment

learnitall left a comment

Choose a reason for hiding this comment

zacharysarah left a comment

Choose a reason for hiding this comment

qmonnet commented Jul 27, 2023

joestringer left a comment

Choose a reason for hiding this comment

zacharysarah left a comment

Choose a reason for hiding this comment

joestringer commented Jul 31, 2023

qmonnet commented Aug 1, 2023 • edited

qmonnet commented Aug 1, 2023

qmonnet commented Jul 26, 2023 •

edited

qmonnet commented Aug 1, 2023 •

edited