docs_reviewing.md

Joe's SIG Docs review process

• Go through all the open PRs.
  • I generally start with the pull request index and just open tabs for each one, and knock them down tab-by-tab.

The general review process I use:

• make sure they’ve had something respond something to them, even if it’s just assigning yourself to come back to it. Especially for new names, or folks first PR (often indicated by an unsigned CLA) I try to thank them or otherwise interact positively.

  • if the CLA isn’t signed, make a comment referencing the author with @<name> to send a message back asking them to sign the CLA so we can accept the PR.

• review where the PR is being applied

  • if it's under /blog - it's worth a look, but there's a separate set of reviewers for those posts. They appreciate comments on improving wording, validation of images rendering, etc, but no /approve or /lgtm will move that forward (intentionally) - and they don't need to comply with the overall docs site style guide, so the language can be (and is expected to be) less structured and far more informal (and possibly date/time) oriented.
  • if it's under /docs/reference/generated or docs/reference/setup-tools/kubeadm/generated, then politely decline the PR unless it's a regeneration of the generated content from an upstream fix. The content here needs to be resolved in the source-code, and we don't want to accept fixes that will just get overwritten on the next generation run.
  • if it's under /cn/ then decline and request the reviewer apply the fixes to the chinese localization effort repository (this is hopefully temporary)

• run through the current state of the PR; if it’s a quick & easy spelling fix, then I mark /approve && /lgtm right then and there, get it off the plate and done.

• check technical review & editorial review - this is the part that can take the most time.

  • I open the style guide on the side and then read through all the content with a mental checklist of the items in the style guide.
  • I start a review, make suggestions on a line by line basis for improved wording, suggestions, etc.
  • When it’s all said and done, if there are changes I’d like to see, I mark it as “changes requested” in the review and submit it.
  • if there are things I’m not sure of technically, I find a technical reviewer to ask - I don’t have an easy list, but I generally try to guess the relevant SIG and then look up some names from the OWNERS_ALIASES file to ask for assistance.
  • Failing anything there, I use the kubernetes/community repo and look for SIG owners/maintains to ask for help and @name them in the PR, asking for a technical review
  • if the PR is just wrong on some level (blatantly incorrect, bad merge, etc - where you don’t want to see it make progress, or it needs to wait for some reason (waiting on upstream PRs to merge), I drop a /hold on it.
  • I will tag any PRs that are clearly WIP with a do-not-merge/work-in-progress tag
  • if the PR is older and needs a rebase, I'll tag it as such and comment to the PR author about the need.
  • I drop the “Docs Open Issues” to help organize the PRs into ones that are pending updates. That doesn’t get cleared automatically, but it helps when you’ve got a lot of untriaged PR’s pending

• review through all the previous PRs to see if changes happened that would change the impact things, approve and/or lgtm when they’re ready to go

• If you (or someone else on the Docs team) has asked a submitter for updates, then I’ll generally try and reinforce that.

  • or just set aside the PR as “viewed for now” and come back later and see if the submitter has made the requested changes, etc.
  • Sometimes reviews don’t indicate the change requests in the github review, sometimes they do - so it’s hard to always see.

• open the staging deployment and navigate to the changed file to visually inspect the output. Surprisingly small things will mess up the rendering, so it's worth a visual double-check.

• I avoid using /lgtm by itself: I either do it with explicitly adding /approve as well, or I do an /approve by itself when I’m waiting for a technical accuracy review, leaving the approve tag on the PR, and asking the technical reviewing to add the /lgtm to say it’s good to go.

  • You may see a number of pending PR’s that have /approve already in flight and are just waiting for technical reviews to /lgtm to merge

@heckj Thanks for adding these guidelines--this is great! 🌟

Nice Updates! Looks like it is time for me to print a new copy of this as my guiding cheat sheet!