Skip to content

Process improvements for 2023-02-10/17#152

Merged
rptb1 merged 8 commits intomasterfrom
branch/2023-02-10/weekly-pi
Feb 28, 2023
Merged

Process improvements for 2023-02-10/17#152
rptb1 merged 8 commits intomasterfrom
branch/2023-02-10/weekly-pi

Conversation

@rptb1
Copy link
Copy Markdown
Member

@rptb1 rptb1 commented Feb 13, 2023

This is an experiment to batch up process improvements so that they can be reviewed and merged periodically.

The idea is to collect low risk process changes into a single branch so that it can be reviewed and merged with low overheads, preferably using proc.review.express.

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 16, 2023

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 16, 2023

But also, we need a better way to keep track of pending PI!

This observation won't get lost. It's also at

mps/procedure/review.rst

Lines 1115 to 1118 in 936f058

_`.pi.exit`: After action has been taken and recorded on every
suggestion, tell `.role.leader`_. [This procedure doesn't make it
clear how the leader tracks and receives this information, when it
times out, etc. RB 2023-02-01.]

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 17, 2023

Some pending PI:

1. https://github.com/Ravenbrook/mps/pull/141#issuecomment-1433253292

Done in #141 (comment)

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 17, 2023

But also, we need a better way to keep track of pending PI!

For now I've created GitHub label "pending" that we can attach when we come up with PI, so that it doesn't get lost when we "close" an issue or "merge" a pull request. Deliberately a bit vague. I notified @thejayps to include a search for pending stuff in our next weekly scan of open issues. That raises two more things:

  1. Weekly scan (or similar) is not yet documented. We're working on it.
  2. GitHub labels and their uses are not documented, except in GitHub. Can labels transcend GitHub?

Also added proc.review.brainstorm.pending

mps/procedure/review.rst

Lines 946 to 949 in 539c687

_`.brainstorm.pending`: If you record a suggestion, ensure that the
pull request is labelled_ "pending" so that it doesn't get forgotten
when the pull request is closed. [Insert cross-reference to procedure
that will pick it up. RB 2023-02-17]

@rptb1 rptb1 requested review from UNAA008 and thejayps February 19, 2023 17:29
@rptb1 rptb1 marked this pull request as ready for review February 19, 2023 17:29
@rptb1 rptb1 added process To do with process or procedure low risk This work is or would be of low risk of introducing defects. labels Feb 19, 2023
@thejayps thejayps added the essential Will cause failure to meet customer requirements. Assign resources. label Feb 20, 2023
@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 22, 2023

Executing proc.review.entry

  1. Start time 06:20
  2. proc.review.entry.express: Change is small and low risk but nobody is available for express review. Plan to batch with other similar changes on 2023-02-23.
  3. Applying entry.universal
  4. entry.universal.reason: There's no issue but PI actions are explicitly linked.
  5. entry.universal.auto-check: check-rst is flagging errors for reasons that are fixed elsewhere.
  6. Entry passed.
  7. Entry took 4 minutes.

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 22, 2023

Executing proc.review.plan

  1. Start time 06:24.
  2. proc.review.plan.time: Single document change consisting entirely of clarifications. Checking should take no more than 10 mins.
  3. proc.review.plan.rule: rule.generic and rule.style
  4. proc.review.plan.invite: @thejayps , @UNAA008 , and @rptb1 will review on 2023-02-23 11:00.
  5. proc.review.plan.role: Roles will be assigned at kickoff.
  6. Planning took 4 minutes.

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 23, 2023

4. proc.review.plan.invite: @thejayps , @UNAA008 , and @rptb1 will review on 2023-02-23 11:00.

We didn't get to this before @thejayps had to leave today, and since it's changes that @UNAA008 has seen and @rptb1 wrote, we decided to postpone for his fresh eyes. Perhaps proc.review.express later.

@rptb1 rptb1 added the pending Something needs doing, even if closed. label Feb 24, 2023
@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 28, 2023

Rebooting the review!

Executing proc.review.express

  1. Start time 12:15
  2. Logging at 12:30.
  3. proc.review.express.brainstorm: @thejayps forgot that "View File" renders the document exactly as intended. It's like looking at the result of a build as well as the source. A reminder of that could help. @rptb1 That's a useful analogy. @thejayps Also, get people to look at the result first before diffs so they don't get sucked in to reading the diffs. @rptb1 Most defects here were found by me watching someone try to execute my procedure. That could perhaps be a thing in "proc.proc".
  4. Not doing edits right away because we're at 30 mins.
  5. Procedure took 33 mins.

Copy link
Copy Markdown
Contributor

@thejayps thejayps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 minor defects found, 1 question. see inline comments

q: On some occasions @rptb1 had comments removed by github while he was composing them but before submitting them. Is it worth an advisory in this document that comments could be composed in a text editor to reduce the risk of losing work until we understand the circumstances of comment deletion mid-compose better?

Copy link
Copy Markdown
Member Author

@rptb1 rptb1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing proc.review.check.

  1. 3 minor defects

@rptb1 rptb1 self-assigned this Feb 28, 2023
@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 28, 2023

Is it worth an advisory in this document that comments could be composed in a text editor

Reject: It would complicate a lot of procedures with extra details that are just an option for a developer. I think I'll leave it.

@rptb1 rptb1 dismissed thejayps’s stale review February 28, 2023 16:24

proc.review.edit

@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 28, 2023

Executing proc.review.exit

  1. Start time 16:25.
  2. Applied exit.universal.
  3. Revised change passed.
  4. review.exit.calc:
  • hours used: 2
  • hours saved: 8 (maybe more if GitHub don't break our links as a result)
  • major defects remaining: 0
  1. Exit took 5 min.

@rptb1 rptb1 merged commit 0a27a84 into master Feb 28, 2023
@rptb1
Copy link
Copy Markdown
Member Author

rptb1 commented Feb 28, 2023

Executing proc.merge.pull-request

  1. Start time 16:30.
  2. Branch doesn't solve all issues mentioned in the pull request, but it is still labelled "pending" so these won't get lost.
  3. No automated test case is feasible. We can observe how well the procedure works on the next new person.
  4. The merge is not approved by an approving review. Risk very low. Overriding.
  5. Merge took 5 mins.

jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this pull request Sep 17, 2025
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this pull request Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essential Will cause failure to meet customer requirements. Assign resources. low risk This work is or would be of low risk of introducing defects. pending Something needs doing, even if closed. process To do with process or procedure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Various MPS documents contain syntax and reference errors

2 participants