Documentation team meeting 2023-09-11

Development documentation
1

General

  • Check back on PR labels applied by the bot
  • Mini retrospective on meeting format:
    • What works well?
    • What doesn’t work well?
    • What should we improve?
  • @asteflova A lot of review-related labels for PRs, it’s overwhelming. Some could use clarification, some might not be needed at all. My proposal:
    • Needs tech review => Ok
    • Needs review => Needs style review to clarify what kind of review this is
    • Technical review done => Ok
    • Style review done => Ok
    • Not reviewed => Suggestion: Do not use. If a PR not reviewed, then a Need tech/style review should be used.
    • Needs re-review => Suggestion: Do not use. If something needs a re-review, specify the type of the review (tech or style).
    • Needs testing => Tentative suggestion: Do not use. Because who is supposed to do the testing? Shouldn’t testing be part of the reviews?

PR Triage

Anything updated within the past week can be ignored during triage. If someone wants to discuss a specific issue/PR then it should be added to the agenda.

2

Present: @ekohl, @adamlazik1, @mjivraja

General

@ekohl still working on this. The bot is being rewritten and making the old bot work was too much work.

@maximilian is on PTO, postponing by another week.

GitHub label review

  • Needs review => Needs style review to clarify what kind of review this is

This is used for some PRs: Pull requests · theforeman/foreman-documentation · GitHub

@adamlazik1 suggests to change it to “Needs style review”.

It should also be noted that once we use the PR processor bot it will automatically add a label “Not yet reviewed” to any new PR.

  • Not reviewed => Suggestion: Do not use. If a PR not reviewed, then a Need tech/style review should be used.

Doesn’t appear to be used now (Pull requests · theforeman/foreman-documentation · GitHub). Added by @maximilian?

@ekohl and @adamlazik1 agree it can be dropped. Use “needs style/tech review” when needed. The “not yet reviewed” label would also make it redundant.

  • Needs re-review => Suggestion: Do not use. If something needs a re-review, specify the type of the review (tech or style).

This is added because the prprocessor will use this automatically. It is added to any PR that had the label “Waiting on contributor” and new code was pushed, indicating that it is no longer waiting on the author of the PR.

  • Needs testing => Tentative suggestion: Do not use. Because who is supposed to do the testing? Shouldn’t testing be part of the reviews?
    [/quote]

This label used to be applied by the bot, but it no longer is. While it was relevant for code repositories, even there it wasn’t getting any use. There are no PRs with it (Pull requests · theforeman/foreman-documentation · GitHub).

@adamlazik1 and @ekohl agree it should be dropped.

PR Triage

Went over open PRs. We briefly went over one issue, but time ran out.

3

Sorry I missed the meeting.

Great job with the labels! I’ll do a cleanup of the labels.

4

Okay, I’ve done a quick process with the labels, deleted those not needed and added what was required.

I also colour-coded them a bit :slight_smile:

1 Like