diff --git a/contributing.rst b/contributing.rst index 38de05c8c6..fc288363d1 100644 --- a/contributing.rst +++ b/contributing.rst @@ -11,15 +11,22 @@ Review The MPS is highly engineered and rigorously controlled in order to prevent defects. This approach has lead to an extremely small number of -bugs in production since its first commercial use in 1997. There are a -fairly large number of rules, both low- and high-level that your code -must follow in order to be accepted. These rules are the result of -continuous process improvement to prevent defects. Unfortunately, we do -not have many of them published at present. We apologise if you find it -frustrating that we do not accept your changes as they are. +bugs in production since its first commercial use in 1997. + +In order to be accepted your change must pass the `MPS review +procedure`_ (proc.review_). To do so it must follow rules, both low- +and high-level, that can be found in the `procedure directory`_. +These rules are the result of continuous process improvement to +prevent defects. + +We apologise if you find it frustrating that we do not accept your +changes as they are. The style guide in guide.impl.c.format_ contains basic rules for style. +.. _MPS review procedure: proc.review_ +.. _proc.review: procedure/proc.review.rst +.. _procedure directory: procedure/ .. _guide.impl.c.format: design/guide.impl.c.format.txt diff --git a/manual/source/design/index.rst b/manual/source/design/index.rst index 8a7a95a65d..39dca7f4a7 100644 --- a/manual/source/design/index.rst +++ b/manual/source/design/index.rst @@ -37,9 +37,6 @@ Design freelist guide.developer guide.hex.trans - guide.impl.c.format - guide.impl.c.naming - guide.review interface-c keyword-arguments land diff --git a/procedure/entry.design.rst b/procedure/entry.design.rst new file mode 100644 index 0000000000..b29e028e2d --- /dev/null +++ b/procedure/entry.design.rst @@ -0,0 +1,78 @@ +================================================================ +Memory Pool System entry criteria for review of design documents +================================================================ + +:tag: entry.design +:type: rule +:status: incomplete +:author: Gavin Matthews +:organization: Harlequin Limited +:date: 1998-04-14 +:confidentiality: public +:copyright: See `C. Copyright and License`_ +:readership: MPS developers + +_`.scope`: These entry criteria are to be used for reviews of design +documents (`proc.review.entry.criteria`_). + +.. _`proc.review.entry.criteria`: review.rst#51-review-entry + +_`.rfc`: Any design document entering formal review must have +previously been reviewed informally, e.g. by e-mail RFC, or design +brainstorm. [This worked in the MM Group, but in public, by whom? +With what outcome? RB 2023-01-24] + + +A. References +------------- + +.. [Gilb_93] "Software Inspection"; Tom Gilb, Dorothy Graham; Addison + Wesley; 1993; ISBN 0-201-63181-4; book.gilb93. + + +B. Document History +------------------- + +Full document history before to 2001 has been lost because this +document was maintained in "Spring", a Lotus Notes information system +at Harlequin Limited. Ravenbrook Limited did not receive a full +revision history when it acquired the MPS from Harlequin's successor. + +========== ===== ================================================== +1998-04-14 GRM Created. +2001-10-09 NDL Imported to Perforce. +2023-01-24 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +------------------------ + +Copyright © 2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/entry.impl.rst b/procedure/entry.impl.rst new file mode 100644 index 0000000000..4ea8fcff34 --- /dev/null +++ b/procedure/entry.impl.rst @@ -0,0 +1,83 @@ +================================================= +Memory Pool System entry criteria for source code +================================================= + +:tag: entry.impl +:type: rule +:status: incomplete +:author: Gavin Matthews +:organization: Harlequin Limited +:date: 1996-07-24 +:confidentiality: public +:copyright: See `C. Copyright and License`_ +:readership: MPS developers + +_`.scope`: These entry criteria are to be used for reviews of code +(see `proc.review.entry.criteria`_). + +.. _`proc.review.entry.criteria`: review.rst#51-review-entry. + +_`.author`: In the case of code there is often not a clear author. In +this case, the leader may select one or more people who have edited +the file recently, at their discretion. [This is out of place. By +the time entry criteria are being selected, the author role has been +indentified. See proc.review.role.author. RB 2023-01-24] + +_`.auto-check`: Source code must compile without errors. Lint should +have been run where available. [Seems like there are three criteria +here: errors, warnings, and lint. RB 2023-01-24] + + +A. References +------------- + +.. [Gilb_93] "Software Inspection"; Tom Gilb, Dorothy Graham; Addison + Wesley; 1993; ISBN 0-201-63181-4; book.gilb93. + + +B. Document History +------------------- + +Full document history before to 2001 has been lost because this +document was maintained in "Spring", a Lotus Notes information system +at Harlequin Limited. Ravenbrook Limited did not receive a full +revision history when it acquired the MPS from Harlequin's successor. + +========== ===== ================================================== +1996-07-24 GRM Created. +2001-10-09 NDL Imported to Perforce. +2023-01-24 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +------------------------ + +Copyright © 2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/entry.universal.rst b/procedure/entry.universal.rst new file mode 100644 index 0000000000..b3d65f00b1 --- /dev/null +++ b/procedure/entry.universal.rst @@ -0,0 +1,119 @@ +================================================= +Memory Pool System entry criteria for all reviews +================================================= + +:tag: entry.universal +:type: rule +:status: incomplete +:author: Gavin Matthews +:organization: Harlequin Limited +:date: 1996-07-24 +:confidentiality: public +:copyright: See `C. Copyright and License`_ +:readership: MPS developers + +_`.scope`: These entry criteria are to be used for all reviews (see +`proc.review.entry.criteria`_). + +_`.author`: The author_ of the document agrees to the review. + +_`.reason`: The change must include, or permanently link to as a +`source`_, the reason the change is needed, expressed in terms of +requirements. + +- On GitHub, this can be the GitHub issue linked via the pull request. + +_`.reviewable`: It must be feasible to review the change using the +review procedure (`proc.review`_). See `proc.review.plan.tactics`_. + +_`.source-available`: The `source documents`_ are available in +writing. [What is the purpose of "in writing" here and how to express +it now? RB 2023-01-24] + +_`.source-approved`: The `source documents`_ will have `exited review`_. +Failing this, they should be mini-reviewed and the fact noted. +Failing this, they should be marked as "UNREVIEWED". + +_`.rules-available`: The relevant rules are available in writing. + +_`.rules-approved`: The rule documents have exited review. Failing +this, see `.source-approved`_. + +_`.brief-check`: The leader has performed a cursory examination and +has found no more than one major defect (see guide.review.class.major +[Needs importing and reference. RB 2023-01-23]) per page. [This +should go further up the list. RB 2023-01-24] + +_`.plan`: There is a procedure for the review, including checking +rates for this type of document. + +_`.training`: The leader has been formally trained and certified in +Inspection. [This is unrealistic in the public MPS. What can we say? +RB 2023-01-24] + +_`.auto-check`: All automatic checks have been performed on the +product document, and it has passed. [What counts as "all"? This +means all the ones that are implemented in the MPS, e.g. +``make test``, CI checks, checks in ``tool/*``, etc. RB 2023-01-24] + +.. _`proc.review`: review.rst +.. _`proc.review.entry.criteria`: review.rst#51-review-entry +.. _author: review.rst#3-review-roles +.. _source: review.rst#6-documents +.. _source documents: source_ +.. _exited review: review.rst#58-review-exit + + +A. References +------------- + +.. [Gilb_93] "Software Inspection"; Tom Gilb, Dorothy Graham; Addison + Wesley; 1993; ISBN 0-201-63181-4; book.gilb93. + + +B. Document History +------------------- + +Full document history before to 2001 has been lost because this +document was maintained in "Spring", a Lotus Notes information system +at Harlequin Limited. Ravenbrook Limited did not receive a full +revision history when it acquired the MPS from Harlequin's successor. + +========== ===== ================================================== +1996-07-24 GRM Created. +2001-10-09 NDL Imported to Perforce. +2023-01-24 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +------------------------ + +Copyright © 2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/exit.universal.rst b/procedure/exit.universal.rst new file mode 100644 index 0000000000..d71cebda76 --- /dev/null +++ b/procedure/exit.universal.rst @@ -0,0 +1,111 @@ +================================================== +Memory Pool System exit criteria for all documents +================================================== + +:tag: exit.universal +:type: rule +:status: incomplete +:author: Gavin Matthews +:organization: Harlequin Limited +:date: 1996-08-08 +:confidentiality: public +:copyright: See `C. Copyright and License`_ +:readership: MPS developers + +_`.scope`: These exit criteria should be used for all reviews (see +`proc.review.exit.criteria`_). + +.. _`proc.review.exit.criteria`: review.rst#58-review-exit + +_`.edit`: The editor has taken written action on all issues, recorded +in the review record; these may include rejecting the issue. + +_`.quest`: All questions to the author (q) should have been answered +in mail to mm, and possibly in documentation. ["Mail to mm" is +specific to the Harlequin MM Group. GitHub comment responses are now +sufficient. RB 2023-01-24] + +_`.imp`: All improvement suggestions (I) should have resulted in one +of the following: + +- _`.imp.edit`: Edit of another document; not if it is already + approved. + +- _`.imp.mail`: Passed on to someone responsible for the other + document, and accepted. + +- _`.imp.issue`: Escalated to an InfoSys issue. ["InfoSys issue" is + specific to the Harlequin MM Group or Ravenbrook. GitHub issues are + now sufficient. RB 2023-01-24] + +- _`.imp.reject`: Rejected by the editor for a documented reason. + +_`.record`: All fields have been filled in in the review record, +including metrics, and estimated defects remaining. + +_`.defects`: The estimated defects remaining is less than the +acceptable level for the document type. In the absence of a more +specific level, use 2 major defects (see guide.review.class.major +[Needs importing and referencing. RB 2023-01-23]) per page. + +_`.rates`: The checking and logging rates did not exceed the optimum +rates by more that 20% on average. [Being too slow is ok.] + +_`.veto.author`: The author does not wish to veto exit. + +_`.veto.leader`: The leader does not wish to veto exit. + + +A. References +------------- + +.. [Gilb_93] "Software Inspection"; Tom Gilb, Dorothy Graham; Addison + Wesley; 1993; ISBN 0-201-63181-4; book.gilb93. + + +B. Document History +------------------- + +Full document history before to 2001 has been lost because this +document was maintained in "Spring", a Lotus Notes information system +at Harlequin Limited. Ravenbrook Limited did not receive a full +revision history when it acquired the MPS from Harlequin's successor. + +========== ===== ================================================== +1996-08-08 GRM Created. +2001-10-09 NDL Imported to Perforce. +2023-01-24 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +------------------------ + +Copyright © 2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/design/guide.impl.c.format.txt b/procedure/guide.code.c.format.rst similarity index 98% rename from design/guide.impl.c.format.txt rename to procedure/guide.code.c.format.rst index 07e317f66d..ebfc60ec50 100644 --- a/design/guide.impl.c.format.txt +++ b/procedure/guide.code.c.format.rst @@ -3,7 +3,7 @@ C Style -- formatting ===================== -:Tag: guide.impl.c.format +:Tag: guide.code.c.format :Author: Richard Brooksby :Date: 1995-08-07 :Status: complete guide @@ -371,6 +371,9 @@ Document History - 2012-09-26 RB_ Converted to Markdown and reversed inconsistent switch "law". +- 2023-02-26 RB_ Renamed to guide.code.c.format pending merger into + GitHub-based proc.review. + .. _DRJ: https://www.ravenbrook.com/consultants/drj .. _RHSK: https://www.ravenbrook.com/consultants/rhsk .. _RB: https://www.ravenbrook.com/consultants/rb @@ -379,7 +382,7 @@ Document History Copyright and License --------------------- -Copyright © 2002–2020 `Ravenbrook Limited `_. +Copyright © 2002–2023 `Ravenbrook Limited `_. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are diff --git a/design/guide.impl.c.naming.txt b/procedure/guide.code.c.naming.rst similarity index 94% rename from design/guide.impl.c.naming.txt rename to procedure/guide.code.c.naming.rst index 12cf264ff0..62f35f50c1 100644 --- a/design/guide.impl.c.naming.txt +++ b/procedure/guide.code.c.naming.rst @@ -3,7 +3,7 @@ C Style -- naming ================= -:Tag: guide.impl.c.naming +:Tag: guide.code.c.naming :Author: Gareth Rees :Date: 2014-10-07 :Status: incomplete guide @@ -95,14 +95,18 @@ Document History - 2014-10-07 GDR_ Created based on job003693_. +- 2023-02-26 RB_ Renamed to guide.code.c.format pending merged into + GitHub-based proc.review. + .. _job003693: https://www.ravenbrook.com/project/mps/issue/job003693/ .. _GDR: https://www.ravenbrook.com/consultants/gdr +.. _RB: https://www.ravenbrook.com/consultants/rb Copyright and License --------------------- -Copyright © 2002–2020 `Ravenbrook Limited `_. +Copyright © 2002–2023 `Ravenbrook Limited `_. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are diff --git a/design/guide.review.txt b/procedure/guide.code.rst similarity index 90% rename from design/guide.review.txt rename to procedure/guide.code.rst index 27655838aa..9629baac2a 100644 --- a/design/guide.review.txt +++ b/procedure/guide.code.rst @@ -3,7 +3,7 @@ Review checklist ================ -:Tag: guide.review +:Tag: guide.code :Status: incomplete documentation :Author: Gareth Rees :Organization: Ravenbrook Limited @@ -48,15 +48,19 @@ correspond? Example: job003922_. See `.diff`_. Document History ---------------- -2015-08-10 GDR_ Created. +- 2015-08-10 GDR_ Created. + +- 2023-02-26 RB_ Renamed to guide.code.c.format pending merged into + GitHub-based proc.review. .. _GDR: https://www.ravenbrook.com/consultants/gdr/ +.. _RB: https://www.ravenbrook.com/consultants/rb Copyright and License --------------------- -Copyright © 2015–2020 `Ravenbrook Limited `_. +Copyright © 2015–2023 `Ravenbrook Limited `_. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are diff --git a/procedure/index.rst b/procedure/index.rst index 2d78d8c9e1..148602ced0 100644 --- a/procedure/index.rst +++ b/procedure/index.rst @@ -8,21 +8,15 @@ Memory Pool System Product Procedures :revision: $Id$ :confidentiality: public :copyright: See `C. Copyright and License`_ +:readership: anyone 1. Introduction --------------- -This document lists the procedures which are applied to the product -sources of the Memory Pool System project. (Compare with the `procedures -which govern the *project* `__.) +This document lists the procedures of the Memory Pool System project. -This document will be updated as new procedures are created. - -The readership of this document is anyone developing or extending the -product sources. - -This document is not confidential. +This document should be updated when new procedures are created. 2. Procedures @@ -32,11 +26,13 @@ This document is not confidential. `branch-merge`_ Branching and merging for development. `pull-request-merge`_ Pull request merge procedure for GitHub. `release-build`_ Build product releases from the sources. +`review`_ Review a change to prevent defects. `version-create`_ Create a new MPS version branch. ===================== ================================================== .. _branch-merge: branch-merge .. _release-build: release-build +.. _review: review .. _version-create: version-create .. _pull-request-merge: pull-request-merge @@ -45,6 +41,9 @@ expose extensions, as recommended by Tim Berners-Lee. But they don't work on GitHub. We should probably make them work there. RB 2023-01-07] +[FIXME: Update with other documents in the procedure directory. RB +2023-02-26] + A. References ------------- @@ -62,6 +61,7 @@ B. Document History 2014-01-13 GDR_ Added branch-merge. 2020-07-28 PNJ_ Updated licence text. 2023-01-07 RB_ Added pull-request-merge. +2023-01-28 RB_ Added review. ========== ======= ================================================== .. _GDR: mailto:gdr@ravenbrook.com @@ -69,6 +69,7 @@ B. Document History .. _RHSK: mailto:rhsk@ravenbrook.com .. _PNJ: mailto:pnj@ravenbrook.com + C. Copyright and License ------------------------ diff --git a/procedure/review.rst b/procedure/review.rst new file mode 100644 index 0000000000..33ca723ffd --- /dev/null +++ b/procedure/review.rst @@ -0,0 +1,1794 @@ +=================================== +Memory Pool System review procedure +=================================== + +:tag: proc.review +:author: Richard Brooksby +:organization: Ravenbrook Limited +:date: 2023-01-19 +:confidentiality: public +:copyright: See `C. Copyright and License`_ +:readership: MPS developers, [incorporate roles. RB 2023-01-20] +:status: draft + +.. TODO: Consistent terminology for the work under review, rather than + "change", "work", "product document", etc. + +.. TODO: Check against book.gilb93.proc.* and consider dividing + procedures by role. + +.. TODO: Incorporate MM Group checklists from + + and firm up .plan.check as a step. + +.. TODO: More explicit management of checking rates. + +.. TODO: Explicitly incorporate `irreducible errors + `__. + +.. TODO: More specific links to rationale, [Gilb-93]_ etc. for + justification and variation. + +.. TODO: Lift review record advice, specifically GitHub comment + format, into a section. rule.generic.once! + +.. TODO: Check against [Gilb-93]_ + +.. TODO: Check against [CMU-SEI-93-TR-025]_ + +.. TODO: Incorporate NASA experience, e.g. https://archive.org/details/NASA_NTRS_Archive_20090038685/mode/2up + +.. TODO: Fix gender-specific language, such as "man-hours", + "manpower", etc. + +.. TODO: Link .pi.exit to the procedure developed by `GitHub issue + #274 `_. + + +1. Introduction +=============== + +This is the procedure for reviewing a change to the MPS in order to +prevent defects (`.purpose`_). + +This procedure may seem overwhelming at first, but it can be executed +quickly with practice. + +Time to execute: + +- first time, with experienced leader (`.role.leader.experienced`_) + training checkers, on a small low-risk change: 3h [cite + https://github.com/Ravenbrook/mps/pull/117#issuecomment-1405388814 + et seq. RB 2023-01-30] + +- `.express`_ reviews: <30m + +[TODO: Insert further examples and estimates from our records, +especially recent experiences recorded in GitHub pull requests. RB +2023-01-30] + +This procedure requires *training*, preferably by an experienced +review leader (`.role.leader.experienced`_). At the very least, do +not apply this procedure to risky changes without first: + +- reading and understanding the whole document and the related rules + +- practising the procedure on low-risk changes + +The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", +"SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and +"OPTIONAL" in this document are to be interpreted as described in RFC +2119 [RFC-2119]_. They indicate what is requried for the review +procedure to succeed in achieving its `.purpose`_. + +This version of review was created as part of `a project to migrate +from Perforce to Git (and GitHub) +`_, and has details on +conducting review via GitHub. The process is in no way specific to +GitHub, Git, Perforce, or any other tool. + +This procedure was created by incorporating and updating review +process documents from Ravenbrook and the Harlequin MM Group (see +`A. References`_). This document is the result of process +improvements from hundreds of reviews and thousands of hours of +productive effort. The underlying process is largely derived from +"Software Inspection" [Gilb-93]_, which was itself developed from +`Fagan inspection `__, +and incorporates experience and measurement going back to the 1970s. + +The process is an implementation of "Peer Review" (kpa.pr), a key +process area of level 3 of the Capability Maturity Model +[CMU-SEI-93-TR-025]_, but also contributes a great deal to: + +- Defect Prevention (kpa.dp, level 5) +- Process Change Management (kpa.pcm, level 5) +- Quantitive Process Management (kpa.qpm, level 4) + +For further comparisons and criticism, see "Inspection and Other +Review Techniques — a Comparison" [Gilb-93]_ §1.2. + + +2. Purpose +========== + +_`.purpose`: The purpose of the review procedure is: + +1. _`.goal.fix`: find and correct `major defects`_ + +2. _`.goal.prevent`: prevent future defects + +By doing this, review gets required changes into the MPS while +protecting it from expensive defects (see `.rationale`_). + +_`.def.defect`: A defect is a way in which the work does not meet its +requirements. + +_`.def.defect.major`: A major defect is a defect that will probably +have significantly increased costs to find and fix later in the +development process (see `.class.major`_). + +As with any procedure, you MAY vary this one to meet this purpose, but +you SHOULD read the `.rationale`_. + + +3. Review Roles +=============== + +_`.role`: People taking part in review are given *roles*. + +_`.role.two`: All the roles MAY be covered by just two people. It's +RECOMMENDED to have more people. + +_`.role.all`: The leader (`.role.leader`_) MUST ensure that all roles +are assigned to someone, to ensure that everything necessary gets +done. + +_`.role.everyone`: Every person taking part in review SHOULD be +assigned at least one role, to make it clear what they need to do. + +_`.role.leader`: The *leader* organises and plans the review, and +ensures the procedures are executed. The leader is responsible for +managing the process in all respects for productive results. The +author (`.role.author`_) MAY lead, but this NOT RECOMMENDED, and MUST +be avoided if they are not `.role.leader.experienced`_. + +_`.role.leader.experienced`: The leader SHOULD be *experienced*, +scoring 24 or more points on "Self-Assessment Audit of Your +Inspection/Review Process" [Gilb-93]_ pp xi-xv. In any case it is +RECOMMENDED that the leader has received special instruction in review +or inspection. + +_`.role.author`: The *author* is the person responsible for the change +under review (`.doc.product`_). For example, they're the developer +who submitted a pull request. The author SHOULD read +`.advice.author`_. + +_`.role.checker`: A *checker* checks the change (`.doc.product`_) +during review. There MUST be more than one checker. Every person +taking part in a review is usually also be a checker, including the +author. Checkers will be asked by the leader to check with certain +*checking roles* (`.role.check`_) in mind; this is to increase +coverage and reduce duplication of issues. Not every checker needs to +understand everything about a change (e.g. the programming language) +thoroughly. Many kinds of checking are useful. Checkers also take +part defect prevention in `.phase.brainstorm`_. + +_`.role.editor`: The *editor* is the person responsible for acting on +the issues found during review in order to bring the work to review +exit. It's RECOMMENDED that the editor and `.role.author`_ are the +same person. For example, they're the developer who submitted a pull +request and needs to fix it up before it can be merged. + +_`.role.improver`: The *process improver* takes action to prevent +future occurrences of defects found by review. This SHOULD be the same +person as `.role.editor`_ so that they maintain a good understanding +of (and commitment to) process improvement and defect prevention, but +this isn't always possible, e.g. when `.role.author`_ is an outside +contributor. + +_`.role.scribe`: The scribe is the person who records information (not +just issues) during review meetings. They are usually the same person +as `.role.leader`_. During `.phase.check`_, review tools (such as +GitHub) will often allow checkers to record issues as they check, in +which case the scribe MUST ensure that this has been done. The +scribe also records information during other phases, such as how much +time a review took, who was there, who did what, etc. + + +4. Phases +========= + +_`.phase`: This section describes the phases of a review. Each phase +has a procedure. The phases involve varying groups of people +(`.role`_) and have diverse purposes. + +_`.phase.handbook`: This section may be used as a short "handbook" for +people who have learned the procedure. (Compare with "A one-page +inspection handbook" [Gilb-93]_.) + +_`.phase.order`: To review a change, the following procedures are +executed roughly in the order below. + +#. _`.phase.request`: `.role.author`_ requests that their change be + reviewed. For example, they submit a GitHub pull request, or + update the pull request state from "draft" to "ready to review". + +#. _`.phase.entry`: `.role.leader`_ executes `.entry`_. If the change + doesn't meet the entry criteria then the change fails review, and + the rest of the review process is not executed. A `.role.author`_ + who is `.role.leader.experienced`_ MAY do entry on their own work. + +#. _`.phase.plan`: `.role.leader`_ executes `.plan`_ to prepare the + review and arrange for it to happen. + +#. _`.phase.kickoff`: `.role.leader`_ and `.role.checker`_ execute + `.ko`_, beginning the review. + +#. _`.phase.check`: `.role.checker`_ individually execute `.check`_, + according to their checking roles (`.role.check`_), looking for + unique `major defects`_ that no other checker will bring to the + logging meeting. Checking continues during the next phase, + `.phase.log`_. + +#. _`.phase.log`: `.role.leader`_, `.role.scribe`_, and + `.role.checker`_ together execute `.log`_ to share and record what + has been found, and to find more `major defects`_, stimulated by + what has been found so far. `.phase.check`_ continues during this + phase. + +#. _`.phase.brainstorm`: `.role.leader`_, `.role.scribe`_, and + `.role.checker`_, execute `.brainstorm`_ to come up with ways of + preventing defects in future. + +#. _`.phase.estimation`: `.role.leader`_, `.role.scribe`_, and + `.role.checker`_ spend a few minutes using `.calc`_ to estimate how + productive the review was, by: + + - estimating the cost of the review (mostly work hours) + - projecting what the defects would cost if uncorrected + - projecting what similar defects would cost if not prevented + + and `.role.scribe`_ records this information. + + [FIXME: There's nothing in the procedure sections about this. + There needs to be at least a placeholder, so it doesn't get lost. + RB 2023-11-09] + +#. _`.phase.edit`: `.role.editor`_ executes `.edit`_, analysing and + correcting defects, but taking *some* action on *every* issue. + This produces the *revised change* (`.doc.rev`_). + +#. _`.phase.pi`: `.role.improver`_ executes `.pi`_ to prevent `major + defects`_ by correcting *causes*. + +#. _`.phase.exit`: `.role.leader`_ executes `.exit`_. If the revised + change does not meet the exit criteria then it fails review. + Otherwise it passes and may go on to be used, e.g. by being merged + into the master codeline (`proc.merge.pull-request`_). + +Even the express review procedure (`.express`_) has these phases. + +.. _proc.merge.pull-request: pull-request-merge.rst + +.. _major defects: `.def.defect.major`_ + + +5. Procedures +============= + +5.1. Review Entry +----------------- + +_`.entry`: The *review entry procedure* MUST be executed when a +change is submitted for review (`.phase.entry`_). + +_`.entry.purpose`: The purpose of entry is to check whether the change +is ready for review before planning a review, committing resources, +organizing meetings, etc. + +_`.entry.express`: Does this change look low risk? Is someone +available? Consider the *express review procedure* (`.express`_). + +_`.entry.record`: Record the entry procedure (`.doc.record`_). + +- On GitHub, you SHOULD start a comment on the pull request. + +- Record a the procedure you're following (this one) and the start + time. Use a permalink_. For example:: + + Executing [proc.review.entry](https://github.com/Ravenbrook/mps/blob/d4ef690a7f2a3d3d6d0ed496eff46e09841b8633/procedure/review.rst#51-review-entry) + + 1. Start time 11:03. + +_`.entry.change`: Record exactly what the change is. + +- On GitHub, this information is implicitly recorded by commenting on + the pull request in `.entry.record`_. + +- Otherwise, record something like the branch name and commit hash. + [Note: That's not really enough. See `GitHub issue #273 + `_. RB 2023-11-09] + +_`.entry.criteria`: Determine and record the entry and exit criteria. + +- `entry.universal`_ and `exit.universal`_ always apply. + +- Add criteria for the types of documents altered by the change (code, + design, etc.) from the `procedure directory`_. + +- Record permalinks_ to the criteria. For example:: + + Executing [proc.review.entry](https://github.com/Ravenbrook/mps/blob/d4ef690a7f2a3d3d6d0ed496eff46e09841b8633/procedure/review.rst#51-review-entry) + + 1. Start time 11:03. + + 2. Applying [entry.universal](https://github.com/Ravenbrook/mps/blob/eceaccdf5ab8d8614e9a8bb91a23bdcb99e7d0ce/procedure/entry.universal.rst) and [entry.impl](https://github.com/Ravenbrook/mps/blob/eceaccdf5ab8d8614e9a8bb91a23bdcb99e7d0ce/procedure/entry.impl.rst). + +.. _permalinks: permalink_ + +_`.entry.check`: Check that the entry criteria hold. Record any +transgressions. Decide whether to reject the change from review by +balancing `.purpose`_ and cost. Will it pass `.exit`_? + +_`.entry.metrics`: Record the time taken to execute `.entry`_. + +.. _entry.universal: entry.universal.rst + +.. _exit.universal: exit.universal.rst + +.. _procedure directory: ./ + + +5.2. Review Planning +-------------------- + +_`.plan`: The *review planning procedure* MUST be executed when +a change has passed `.entry`_. + +_`.plan.purpose:` The purpose of planning is to prepare the review so +that it is efficient and effective, and arrange for it to happen. + +_`.plan.record`: Record the planning procedure. + +- On GitHub, start a `pull request comment`_. + +- Record the procedure you're following (this one) and the start time. + Use a permalink_. For example:: + + Executing [proc.review.plan](https://github.com/Ravenbrook/mps/blob/d4ef690a7f2a3d3d6d0ed496eff46e09841b8633/procedure/review.rst#52-review-planning) + + 1. Start time 11:31. + +_`.plan.iterate`: Consider all of this procedure. + +- This procedure is only in rough order. Later steps may change + earlier decisions. For example, availability of people for + `.plan.roles`_ might affect `.plan.tactics`_. + +_`.plan.identify`: Identify the change to be checked. This is often +quite simply the documents altered by the change, but might need to +expand to include dependent documents, or even things outside the +project. + +_`.plan.tactics`: Examine the change and decide how to check it to +achieve the `.purpose`_. + +- The default and most effective tactic is to have `.role.checker`_ + examine every line of the change, evenly distributing their + attention by using a checking rate, such as 10 lines/minute. + +- Large repetitive automatic changes (search-and-replace) might be best + handled by sampling using a random number generator and a strong + Brownian motion producer (dice and tea). + +- Large changes might be broken up by document type, or topic, but you + still want multiple `.role.checker`_ to look at everything. + +- Changes that cannot feasibly be checked + (`entry.universal.reviewable`_) may need to be reworked into stages, + perhaps by version control transformations. (For example, the + prototype branch/2014-02-19/remember-time was reworked into + branch/2014-04-14/remember-time-2, + branch/2016-03-22/remember-time-3, branch/2018-08-08/refset-struct + etc.) + +- Record any variations from the default tactic. + +.. _entry.universal.reviewable: entry.universal#reviewable + +_`.plan.time`: Estimate the checking rate and time. + +- GitHub provides diff stats on the pull request (to the right of + "Conversation"). + +- `.phase.check`_ SHOULD last no more than one hour, so that checkers + can maintain concentration. + +- `.phase.log`_ SHOULD last no more than two hours, so that checkers + can maintain concentration. + +- It may be necessary to divide the review into multiple sessions. + +- Record your estimates. For example:: + + Executing [proc.review.plan](https://github.com/Ravenbrook/mps/blob/d4ef690a7f2a3d3d6d0ed496eff46e09841b8633/procedure/review.rst#52-review-planning) + + 1. Start time 11:31. + + 2. proc.review.plan.time: About 500 lines of code @ 10 lines/minute + so about 50 mins of checking. + +_`.plan.schedule`: Plan when this review may take place and who will +attend. Negotiate with attendees if appropriate. + +- Record like:: + + 3. proc.review.plan.schedule: @thejayps and @UNAA008 will review + 2023-01-23 11:00 for about 2h. + +_`.plan.train`: Ensure that all participants are familiar with the +review process. + +- Brief anyone new to the process about how it works and what is + expected of them. + +- Ensure that they have the process documents. + +- Allow extra time for training. + +_`.plan.source`: Determine and record the source documents that could +be used for checking (`.doc.source`_). + +- Always include issues resolved or partially resolved by the change. + There SHOULD be at least one (ensured by `.entry.criteria`_). + +- Consider requirements, issues, designs, analysis, discussions, + records of failures (e.g. in email messages), user documentation, + standards. + +_`.plan.rule`: Determine and record the rules to apply (`.doc.rule`_). + +- Add rules for the types of documents altered by the change (code, + design, etc.) from the `procedure directory`_. + +- Also select other rules that apply from the `procedure directory`_, + for example special rules that apply to the critical path. [These + do not exist yet. RB 2023-11-09] + +_`.plan.check`: If there are relevant checklists available, determine +and record which ones to apply. + +_`.plan.roles`: Decide and record the checking roles (`.role.check`_) +to assign. + +- Consider and try to assign every checking role (`.role.check`_). + +- Choose checking roles that are most likely to find `major defects`_ + in the type of change under review. + +- Always try to assign `.role.check.backwards`_ or a similar + out-of-order sampling method, to help find defects in all parts of + the change. + +- Bear in mind that `.role.leader`_ and `.role.scribe`_ will be + somewhat occupied during logging and less able to check. + +- Assignments can be renegotiated in `.ko.role`_. + +_`.plan.homework`: Assign work that people should do before the +review. + +- Include background reading or other self-education that will help + review efficiency. For example, reading about a technical aspect of + the change. + +- You SHOULD NOT request review activities like studying source + documents or looking at the change. Plan properly. + +- Ask anyone new to the process to read this document (see + `.plan.train`_). + +- Plan the review to succeed (but perhaps take longer) even if the + homework is not done. + +_`.plan.invite`: Invite the checkers (`.role.checker`_) to the kickoff +meeting (`.ko`_). + +_`.plan.doc`: Ensure that `.role.checker`_ have all the documents they +need (the change, source documents, rules, etc.) + +_`.plan.metrics`: Record the time taken to execute `.plan`_. + + +5.3. Review Kickoff +------------------- + +_`.ko`: `.role.leader`_ holds the *review kickoff* meeting to ensure +that the review begins, and that everyone involved has what they need +to perform their roles. + +_`.ko.record`: Record the kickoff procedure. + +- On GitHub, start a `pull request comment`_. + +- Record the procedure you're following (this one) and the start time. + Use a permalink_. For example:: + + Executing [proc.review.kickoff](https://github.com/Ravenbrook/mps/blob/b2050e2cf69029fc13c31a724421945952d3fab2/procedure/review.rst#53-review-kickoff) + + 1. Start time 15:00. + +_`.ko.doc`: Ensure that every checker has all the documents they need. + +_`.ko.intro`: Optionally, ask the author for a short (one minute) +introduction to the change. + +- Everyone should listen for new information this reveals. Start the + `.log.record`_ early if there's anything that needs documenting, + such as a hidden assumption or requirement. This happens! + +_`.ko.remind`: The leader reminds everyone: + +- of the `.purpose`_ of review + +- that they are trying to find unique `major defects`_ not found by + other checkers + +- they are not restricted to finding defects caused by `.doc.product`_ + +- not to confer until `.log`_ + +- to check the document *as it will be after the change* and not only + check diffs (`.check.diff.not`_), and especially to not get + distracted by the "Files changed" tab in a GitHub pull request + +- to open and apply `proc.review.check`_ + +- to avoid finishing `pull request reviews`_ or submitting `pull + request comments`_ until `.log`_. + +.. _pull request reviews: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews + +.. _pull request comments: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#about-pull-request-comments + +.. _proc.review.check: `.check`_ + +_`.ko.role`: Negotiate checking roles (`.role.check`_). + +- `.role.checker`_ may volunteer for roles based on how they feel at + the time. Focus and enjoyment are important for good results. + +- Ensure checkers understand their checking roles and checking rates. + +- Record who's doing what. + +_`.ko.train`: Offer private help to new `.role.checker`_ after `.ko`_ +so that you don't delay `.check`_. + +_`.ko.improve`: Announce any review metrics and negotiate review +objectives. + +- Announce the rate. + +- Ask for suggestions or experiments with review procedure. + +- Record metrics and objectives. + +_`.ko.log`: Set a time for the logging meeting (`.log`_). + +- This SHOULD be set at the estimated end of `.ko`_, plus the + estimated checking time (see `.plan.time`_), plus a short break. + Avoid delay. + +_`.ko.author`: Remind the author that they can withdraw the document +from review at any time. + +_`.ko.go`: Send `.role.checker`_ away to start `.check`_. + +_`.ko.metrics`: Record the time taken to execute `.ko`_. + + +5.4. Review Checking +-------------------- + +_`.check`: The *checking procedure* SHOULD be executed by each +individual `.role.checker`_ alone, carrying out their assigned +checking roles (`.role.check`_) without conferring with other +checkers. + +_`.check.purpose`: The purpose of checking is to find unique `major +defects`_ that no other checker will bring to `.log`_. + + +5.4.1. Start +............ + +_`.check.doc`: Ensure that you have all the documents you need to +perform your checking role (`.role.check`_). + +_`.check.ask`: Ask `.role.leader`_ if you have any questions about +checking. + + +5.4.2. Checking +............... + +_`.check.record`: You may note what you find in any way you like. + +_`.check.record.start`: Make a note of the start time. + +_`.check.record.github`: You SHOULD note issues using GitHub's `pull +request reviews`_ in a way that will save time during `.log`_. + +- You SHOULD use `.log.format`_, e.g. "M: assumes array size, + rule.imple.assume", but concentrate on `.check.major`_. + +_`.check.diff.not`: You SHOULD NOT check using diffs unless your +checking role says so. Check the work *as it will be after the +change* only using the diffs to help direct attention. + +_`.check.source`: Read `.doc.source`_ for your `.role.check`_. + +- Don't spend time searching for defects in `.doc.source`_. If you + happen to find any, that's a bonus. Note them for logging as + `.class.imp`_ and possibly `.class.major`_ as well. + +_`.check.rule`: Ensure that you know `.doc.rule`_ and `.doc.check`_. + +- If they've changed since you last read them, study and understand + the changes. + +_`.check.role`: Ensure that you know `.role.check`_ and keep it in +mind as you check. + +_`.check.product`: Check `.doc.product`_. + +_`.check.rate`: Try to check at the planned checking rate +(`.plan.time`_). Do not rush. Slower is usually better. Control +your attention. + +_`.check.major`: Concentrate on finding `major defects`_. + +_`.check.max`: Find as many issues as possible to help the author. + +_`.check.note`: Note all issues; you need not log them later. + +_`.check.rough`: Your notes can be rough. `.check.major`_! + +- Do not spend time making your issues neat and clear or even putting + them in exactly the right place. Save that for `.log`_. Search for + more issues. `.check.major`_! + +_`.check.trouble`: Consult `.role.leader`_ if you're having trouble: + +- you have questions +- you are finding too many or too few issues + +_`.check.class`: Classify each issue you find (`.class`_). + + +5.4.3. End +.......... + +_`.check.metrics`: At the end of checking, record + +- how many issues you found, by class (see `.check.class`_) + +- how long you actually spent checking + +- how much of the product document you actually checked + +- any problems encountered + +_`.check.metrics.github`: You MAY record your metrics in a GitHub +`pull request review`_. + +#. Open the "Files changed" tab of the pull request. + +#. Click the green "Review changes" button. + +#. Enter metrics in the text box. + +#. Do not "finish" your review before `.log`_ to avoid distracting + other `.role.checker`_. + +.. _pull request review: `pull request reviews`_ + + +5.5. Review Logging +------------------- + +_`.log`: The *review logging procedure* is executed by `.role.leader`_ +and `.role.scribe`_ together with `.role.checker`_. + +_`.log.purpose`: It has two purposes: + +1. to record issues for action + +2. to find more `major defects`_, stimulated by sharing what has been + found so far + +_`.log.check`: Checking continues during logging. + +_`.log.advice`: Remind the author of `.advice.author`_. + +_`.log.author`: Remind the author that they can withdraw +`.doc.product`_ from review at any time. + +_`.log.record`: `.role.scribe_` MUST record the logging procedure. + +- On GitHub, start a `pull request comment`_. + +- Record the procedure you're following (this one) and the start time. + Use a permalink_. For example:: + + Executing [proc.review.log](https://github.com/Ravenbrook/mps/blob/12160d613b04045d6bd5380932f7560c91647556/procedure/review.rst#55-review-logging) + + 1. Start time: 15:50. + +- This opens `.doc.log`_. `.role.scribe`_ MAY append issues to the + log, but see `.log.record.github`_. + +.. _permalink: https://docs.github.com/en/repositories/working-with-files/using-files/getting-permanent-links-to-files + +.. _pull request comment: `pull request comments`_ + +_`.log.record.github`: Ask `.role.checker`_ using the GitHub review +tool to "finish" their reviews now, so that their notes and metrics +are automatically included in `.doc.log`_. `Major defects`_ recorded +in this way MUST still be "logged" by announcing them to the meeting +(`.log.major`_). + +_`.log.sums`: "How many did you find?" Gather, sum, and record +individual metrics from `.check.record`_ of: + +- how many issues were found, by class (see `.check.class`_) + +- how long was spent checking + +- how much of the product document was checked + +_`.log.decide`: Now, and at intervals during logging, assess whether +`.doc.product`_ is likely to pass `.exit`_. If it seems very +unlikely, consult with `.role.author`_ and `.role.editor`_ about +aborting the logging meeting. Bear in mind: + +- Second reviews often find fewer issues, so it may be worth logging + them anyway. + +- `.brainstorm`_ needs `major defects`_ to work on, and might prevent + whatever went wrong here. + +- The MM Group never aborted logging. + +_`.log.plan`: Use the metrics to decide a logging rate. + +- The RECOMMENDED rate is at least one per minute. (This comes from + MM Group experience. See [Gilb-93]_ §5.2.4 for detailed advice.) + +- Try to get all issues are logged during scheduled meeting time. + +- Slow down if many new issues are being found. Speed up if not. + `.role.checker`_ will tell you when they find issues + (`.log.new`_). + +- Schedule breaks to maintain concentration. + +- Consider scheduling more logging meetings. + +_`.log.scribe`: Assign `.role.scribe`_ (usually the leader), and +ensure `.role.editor`_ will find and be able to read the log. + +_`.log.explain`: `.role.leader`_ ensures `.role.checker`_ understand +the order in which issues will be logged. + +_`.log.format`: `.role.leader`_ ensures `.role.checker`_ understand +the desired form of issues, namely: + +- location + +- `.class`_, including `.class.new`_ if the issue was discovered + during logging + +- how it breaks which `.doc.rule`_ or `.doc.check`_, if known, + otherwise briefly what's wrong ("typo", "uninitialized", "obi-wan", + "missing requirement", etc.) + +_`.log.dup`: `.role.leader`_ MAY remind `.role.checker`_ to avoid +logging issues that have are duplicates of ones already logged. + +_`.log.order`: Ask `.role.checker`_ to try to list their issues in +forwards document order. This makes life easier for other checkers +and the editor. (There has been much experimentation with the order +of logging, but was most effective the MM Group.) + +_`.log.major`: `.role.leader`_ calls upon `.role.checker`_ in turn to +announce `major defects`_ they found. + +_`.log.scribble`: `.role.scribe`_ ensures that `major defects`_ are +recorded so that they can *all* be found by `.edit`_ and `.pi`_. + +- On GitHub, the scribe SHOULD edit `.role.checker`_ comments for + `.log.format`_, and MAY + + - start a new GitHub `pull request review`_ to record `.class.new`_ issues + + - make `line comments`_ from the diffs + + - append to `.log.record`_ + + - enter more `pull request comments`_. + +.. _line comments: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request#adding-line-comments-to-a-pull-request + +_`.log.fast`: Log issues briskly. Allow people to clarify the issue, +but discourage discussion. Encourage the search for more `major +defects`_. `.role.leader`_ should firmly discourage discussion of: + +- whether issues are genuine defects + +- how a defect may be resolved + +- the review process (other than to answer questions); + +- the answers to questions logged + +_`.log.slow`: Log issues slowly enough that `.role.checker`_ have time +to understand issues and use them to find more `major defects`_. + +_`.log.new`: When `.role.checker`_ find new `major defects`_ they +should: + +- tell `.role.leader`_, for counting + +- note them as they did during `.check`_ and announce them later in + `.log.major`_ but ensure they are classified as "new" + (`.class.new`_). + +_`.log.decide.non-major`: After logging `major defects`_, decide +whether and how many minor issues (`.class.minor`_) to log during the +meeting, considering `.log.purpose`_. + +- Avoid fatigue. + +- `.role.checker`_ may have already noted minor issues in a way that + can be found during `.edit`_, such as in a `pull request review`_. + +- Perhaps ask `.role.checker`_ to cherry-pick a fraction of their + minor issues and submit the rest later. + +- `.role.checker`_ should cherry-pick issues that have the best chance + of helping to find `major defects`_ or prevent them via + `.brainstorm`_. + +_`.log.non-major`: Go through `.doc.product`_ in sections (or +equivalent), at each stage announce the section, ask who has issues, +and request the issues. + +- `.role.scribe`_ ensures the issues are recorded (see `.log.major`_). + +- This is a good time to log `.class.imp`_ (issues outside + `.doc.product`_) that came up while reviewing specific parts of + `.doc.product`_. + +_`.log.general`: Ask `.role.checker`_ in turn for any general or new +issues not already logged. + +- `.role.scribe`_ ensures the issues are recorded (see `.log.major`_). + +_`.log.brainstorm`: Negotiate a time for the `.brainstorm`_. This +will normally be after a break at the end of `.log`_. + +_`.log.inform`: Inform `.role.editor`_ that `.doc.product`_ is ready for +`.edit`_. + +_`.log.metrics`: Record the time taken to execute `.log`_. + + +5.6. Review Brainstorm +---------------------- + +_`.brainstorm`: The *review brainstorm procedure* should be executed +by `.role.leader`_ with `.role.scribe`_ and `.role.checker`_ very soon +after `.log`_. + +_`.brainstorm.purpose`: The purpose of review brainstorm is to come up +with ways of preventing defects in future (`.goal.prevent`_). + +_`.brainstorm.time`: The process brainstorm should last no more than +around 30 minutes. + +_`.brainstorm.record`: Record the brainstorm procedure +(`.doc.record`_). + +- On GitHub, start a `pull request comment`_. + +- Record a the procedure you're following (this one) and the start + time. Use a permalink_. For example:: + + Executing [proc.review.brainstorm](https://github.com/Ravenbrook/mps/blob/branch/2023-01-19/review-procedure/procedure/review.rst#56-review-brainstorm) + + 1. Start time: 16:33. + +_`.brainstorm.choose`: Choose 3 to 6 `major defects`_ or groups +of `major defects`_ found in review. + +- Make this choice based on defect importance and your experience of + which defects can be most profitably attacked. + +- Record the issues, e.g. by pasting links into the record comment, so + you can edit them for `.brainstorm.log`_. + +_`.brainstorm.remind`: Remind everyone of `.brainstorm.purpose`_ and +`.pi.scope`_. + +_`.brainstorm.focus`: Ask everyone *not* to spend time analysing the +defects found by the review, or suggesting ways to fix those defects, +except insofar as it is necessary to develop ways to *prevent* those +defects. + +_`.brainstorm.raise`: Raise each major defect in turn, reminding +participants of the issue, and asking how it happened and what could +have prevented it. + +_`.brainstorm.disc`: Encourage discussion for no more than about five +minutes per defect. Focus on how the defect arose, and what +improvement could prevent it. Curtailing discussion of how the defect +might be fixed. + +_`.brainstorm.new`: If `.role.checker`_ find new `major defects`_ do +the same as `.log.new`_. + +_`.brainstorm.proc`: If time permits, the leader may solicit +criticisms of the review process and apply `.brainstorm.disc`_ to +them. + +_`.brainstorm.log`: Record the suggestions like:: + + 2. For https://github.com/Ravenbrook/mps/pull/117#discussion_r1090530823 + : @thejayps suggests a checklist item perhaps, where you + deliberately try to misinterpret your sentences and improve + them if you can (misinterpret them). + +_`.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. These are picked up during regular +management of pull requests by `proc.regular`_. + +.. _proc.regular: https://info.ravenbrook.com/project/mps/doc/2023-03-03/regular-tasks/proc.regular + +.. _labelled: https://github.com/Ravenbrook/mps/issues/labels + +_`.brainstorm.metrics`: Record the time taken to execute `.brainstorm`_. + + +5.7. Review Edit +---------------- + +_`.edit`: The *review edit procedure* MUST be executed by +`.role.editor`_ to revise `.doc.product`_ into `.doc.rev`_ by +processing `.doc.log`_. + +_`.edit.purpose`: The purpose of the review edit is to analyse and +correct defects, part of the review's primary purpose (`.goal.fix`_). + +_`.edit.record`: Record the edit procedure. + +- On GitHub, start a `pull request comment`_. + +- Record the procedure you're following (this one) and the start time. + Use a permalink_. For example:: + + Executing [proc.review.edit](https://github.com/Ravenbrook/mps/blob/f8b6c94be9304d017d8a5cf57f7f4ab367ac51fc/procedure/review.rst#57-review-edit) + + 1. Start time 2023-02-02 09:45. + +_`.edit.record.time`: Edit might take several sessions. Keep track of +the working time spent for `.edit.metrics`_. + +_`.edit.read`: Locate and read all of `.doc.log`_ before making any +edits. + +- On GitHub, `.doc.log`_ should be visible as comments, reviews, and + conversations_ on the pull request, starting at the kickoff record + (`.ko.record`_). + +.. _conversations: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews#discovering-and-navigating-conversations + +_`.edit.log`: Record your actions in one of these ways (in order of +preference): + +- Respond to the issue like a conversation. This works well for + GitHub review conversations_. + +- Quote the text of the issue in a comment. This works well for + issues in `pull request comments`_. + +- Edit the log and record your action in a comment, e.g. :: + + m: Warthog too warty. [Fixed: Warts reduced in f93b75dc] + +- Append your action to the `.edit.record`_ with a reference. + +- In any case, your actions MUST be recorded permanently in a way that + is traceable from `.doc.log`_. + +_`.edit.act`: You MUST take action on every issue in `.doc.log`_ and +record that action. Record one of the following responses: + +_`.edit.act.fix`: Fix the defect and say a few words about how. +Always say where. + +- Write "Fix: in " + +_`.edit.act.reject`: Reject the issue with a reason why it is not a +valid issue. + +- Write "Reject: " + +_`.edit.act.comment`: Add a comment to `.doc.product`_ rather than +"fixing" the issue. Say why the issue cannot be fixed. Note that +this is not the same as fixing a defect in a comment. + +- Write "Comment: in " + +_`.edit.act.raise`: Escalate for later action, usually by creating an +issue to go into the project queue, such as a `GitHub issue`_. + +- Write "Raise: " + +- This can apply to `.class.question`_ if it a difficult one. + +.. _GitHub issue: https://docs.github.com/en/issues + +_`.edit.act.forget`: Decide that the issue is not worth an action, +even though it's valid. Give your reason. + +- Write "Forget: " + +- Use with caution, and *never* for `.class.major`_. + +_`.edit.act.answer`: For `.class.question`_, give an answer, and tag +or message the questioner so that they see it. + +- Write "Answer: " + +- You MAY send an answer by some other traceable means and link it. + +_`.edit.act.imp`: Pass the issue to another person, and ensure they +accept it. + +- Write "Pass: " + +- Mainly intended for `.class.imp`_, where some outside document needs + an edit. + +_`.edit.extra`: You may make corrections to defects which you spot +yourself during editing work. Log them like those found during +`.check`_ or `.log`_ and inform `.role.leader`_ about them. + +_`.edit.exit`: After action has been taken and recorded on every +logged issue, tell `.role.leader`_ that the revised change is ready +for `.exit`_. + +_`.edit.metrics`: Record both the working time spent and the end time +of `.edit`_. + + +5.8. Process Improvement +------------------------ + +_`.pi`: The *process improvement procedure* MUST be executed by +`.role.improver`_ to take action to prevent future defects by +processing `.doc.log`_, but especially the results of +`.brainstorm`_. + +_`.pi.purpose`: The purpose of process improvement is to take action +to prevent future defects, closing the process improvement loop +(`.goal.prevent`_). + +_`.pi.scope`: The scope of actions that might be taken by the improver +should not be limited, and could include: + +- filing process issues for later action +- changing a personal habit +- raising concerns with management +- sending suggestions to anyone +- suggesting wholesale review of working practices +- requesting training for staff. + +as well as changes like: + +- adding rules (`.doc.rule`_) or checklist items (`.doc.check`_) +- updating procedures (`.doc.proc`_) +- updating or writing guides (`.doc.guide`_) +- creating tools +- adding automated checks + +_`.pi.record`: Record the process improvement procedure. + +- On GitHub, you start a `pull request comment`_. + +- Record the procedure you're following (this one) and the start time. + Use a permalink_. For example:: + + Executing [proc.review.pi](https://github.com/Ravenbrook/mps/blob/f8b6c94be9304d017d8a5cf57f7f4ab367ac51fc/procedure/review.rst#58-process-improvement) + + 1. Start time 2023-02-02 11:45. + +_`.pi.record.time`: See `.edit.record.time`_. + +_`.pi.read`: Locate and read all of the suggestions recorded in +`.brainstorm.log`_ before making any decisions. + +_`.pi.log`: Record your actions in the same manner as edit actions +(`.edit.log`_). + +_`.pi.act`: You MUST take a written action for every improvement +suggestion and record that action. Record your response like an edit +(`.edit.act`_). + +_`.pi.exit`: After action has been taken and recorded on every +suggestion, tell `.role.leader`_. + +_`.pi.metrics`: See `.edit.metrics`_. + + +5.8. Review Exit +---------------- + +_`.exit`: The *review exit procedure* is MUST be executed by +`.role.leader`_ after editing (`.edit`_). + +_`.exit.purpose`: The purpose of exit is to determine whether the +revised change passes review. + +_`.exit.record`: Record the exit procedure (`.doc.record`_). + +- On GitHub, you start a `pull request comment`_. + +- Record a the procedure you're following (this one) and the start + time. Use a permalink_. For example:: + + Executing (proc.review.exit)[https://github.com/Ravenbrook/mps/blob/645200a25e5e415a2a2978d550b5251e0284c43e/procedure/review.rst#58-review-exit] + + 1. Start time 10:20. + +_`.exit.check`: Check that the exit criteria hold (see +`.entry.criteria`_). + +- Record any transgressions, like:: + + 2. exit.universal.quest: Question 5 answered in chat but not in docs. + +_`.exit.fix`: Fix transgressions, if it is feasible with low risk. +Otherwise ask `.role.editor`_ to fix them. Record this action, and +record edits in the same way as `.edit`_. + +_`.exit.fail`: If transgressions remain, then the revised change is +too defective. It fails review and MUST NOT be used. + +- Record this result, like:: + + 3. Revised change rejected. + +- Tell project management, because they need to decide how to handle + the situation. + +_`.exit.pass`: Otherwise, the revised change passes review and the +resulting `.doc.acc`_ MAY be used. + +- Record this result, like:: + + 3. Revised change passed. + +- On GitHub, `approve the pull request`_ for merge. + +- Tell the person who will deploy `.doc.acc`_, such as someone who + will merge it to master. + +.. _approve the pull request: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/approving-a-pull-request-with-required-reviews + +_`.exit.calc`: Calculate and record final review metrics (`.calc`_). +For example:: + + 4. review.exit.calc: + - hours used: 11 + - hours saved: 70 + - major defects remaining: 1.5 + +_`.exit.inform`: Inform all review participants of the result of their +efforts. + +_`.exit.metrics`: Record the time taken to execute `.exit`_. + + +6. Documents +============ + +[Sourced from [MM-process.review]_ and needs updating. RB 2023-01-21] + +_`.doc`: The review process involves a lot of documents. This is a +brief explanation of what they are. + +_`.doc.forms`: Documents come in many forms. They might be web pages, +email messages, GitHub comments, chat messages, and sometimes even +printed on dead trees. + +_`.doc.source`: Source document + A document from which the product document is derived. Note that + this does not mean "source code". + + For example, a failure of the software might result in a *failure + report*, which gets logged to an *issue*, where someone writes an + *analysis* and *designs* a solution. All of those things are source + documents for the resulting *change* to be reviewed + (`.doc.product`_). + + Other examples include `.doc.guide`_, manuals, and standards. + +_`.doc.product`: Product document + The document developed from the source documents, and offered for + review. The work under review. The changes under review. The work + product. + + Often, we are reviewing a *change* to the MPS, in the form of a + branch to be merged in order to meet a requirement. In this case, + the product is technically the change, and yet we focus the review + on the MPS as it will be after the change, in order to find defects + that would be introduced (see also `.plan.identify`_). The + admonition `.check.diff.not`_ is a manifestion of this principle. + +_`.doc.record`: Review records + Documents produced by the review procedures, which record the + progress and results of the review. See `.entry.record`_, + `.plan.record`_, `.ko.record`_, `.check.record`_, `.log.record`_, + `.brainstorm.record`_, `.edit.record`_, `.pi.record`_, and + `.exit.record`_. + + On GitHub, the record of a review SHOULD consist of `pull request + comments`_, `pull request reviews`_, and `line comments`_. See also + `.doc.log`_. + + In any case, review records SHOULD be specific, permanent, and + MUST be referencable. + +_`.doc.log`: Issue log + A record of issues raised during the logging meeting, specifying + their location, type, finder, and a brief description. + + On GitHub, the issue log includes all GitHub `pull request reviews`_ + or `pull request comments`_ for the change under review. See also + `.doc.record`_. + + Every issue log entry SHOULD be specific, permanent, referencable, and + MUST be traceable from `.doc.product`_ and `.doc.rev`_. + +_`.doc.rev`: Revised document + The result of performing the edit procedure on the `.doc.product`_. + The revised version of the change under review. + +_`.doc.acc`: Accepted document + The result of a Revised document passing exit. + +_`.doc.rule`: Rules and rule sets + A rule or set of rules that `.doc.product`_ should obey. + + Rules are developed by process improvement of the project as a + whole. In this procedure, they are updated by `.pi`_ as a result of + `.brainstorm`_. + + Rule sets are kept short and and rules kept terse to help with + checking. + +_`.doc.guide`: Guides + A guide that `.doc.product`_ is expected to follow, though not + strictly. + + Guides are generally longer, more detailed, and more discursive than + `.doc.rule`_ and contain advice about good practice. As such, they + are less useful for review checking than `.doc.rule`_ or + `.doc.check`_. + + Guides are developed by process improvement of the project as a + whole. In this procedure, they are updated by `.pi`_ as a result of + `.brainstorm`_. + +_`.doc.check`: Checklists + A list of questions to help check against `.doc.rule`_. A negative + answer to a checklist question indicates that a rule has been broken. + + Checklists often contain specific questions that can help determine + whether rules are broken. For example, a code checklist might say + + .error.check: Are function status/error/exception returns + checked and acted upon? + + which is ultimately part of a checking generic rule like + + .achieve: A document must achieve (be consistent with) its + purpose. + + Checklists are developed by process improvement of the project as a + whole. In this procedure, they are updated by `.pi`_ as a result of + `.brainstorm`_. + +_`.doc.entry`: Entry criteria + `.doc.rule`_ that SHOULD be met before review to ensure that the + `.doc.product`_ is likely to pass `.doc.exit`_, so that resources + are not wasted on a premature review. + +_`.doc.exit`: Exit criteria + `.doc.rule`_ that SHOULD be met for `.doc.rev`_ to pass review and be + approved for use. + +_`.doc.proc`: Procedures + Descriptions of the steps involved in completing any part of process + (development, review, or otherwise). + +_`.doc.imp`: Brainstormed improvement suggestions + Suggested improvements to process (and hence to some document) + arising from the process brainstorm. + +_`.doc.request`: Requests for change + An issue that the editor cannot deal with that is escalated to some + other tracking system, such as a `GitHub issue`_. + + +7. Calculations +=============== + +[Update the gender-specific tags in this section. RB 2023-02-02] + +_`.calc`: [Need to mention how this info is used. Ref kpa.qpm. RB +2023-01-26] + +_`.calc.manpower-used`: The manpower used is the time for entry, +kickoff, checking, logging, brainstorm, edit, and exit. Kickoff, +checking, logging and brainstorm should be multiplied by the number of +checkers. Entry and kickoff may be assigned to another document +reviewed at the same time. + +_`.calc.manpower-saved`: The default calculation is the number of +major defects found and fixed, multiplies by 10 man-hours. This +represent the cost of a major defect found by QC. If the defect would +have reached customers, the estimate should be 100 man-hours. A +better estimate can be made, with justification. + +_`.calc.defects-remaining`: The calculation of defects remaining +should use the estimate /. The +obvious adjustment should be made for sampling. The number of +unresolved major issues (raised) should be added. [In an ideal world, +I believe we should know what proportion of major defects we find, and +use that. Perhaps we could use 75%? - GavinM] [Doesn't that mean we +could determine whether a document fails review before `.edit`_? RB +2023-01-28] + + +8. Checking Roles +================= + +["Checking role" is too easily conflated with "review role" and should +perhaps be renamed to "method". RB 2023-01-23] + +_`.role.check`: Checking roles are assigned (`.plan.roles`_) to +`.role.checker`_ in order to focus their attention on different +aspects of the change under review, and so increase the number of +unique major defects found. + +_`.role.check.backwards`: The *backwards checking role* involves +scanning the product document in reverse order, in order to increase +the chances of finding major defects that won't be found by other +checkers. The checker should use their initiative in determining the +granularity of this reversal; for example: in an implementation, the +checker might read each function or type definition in turn from the +end of the file; for other documents, the checker might read each +subsection or paragraph from the end backwards. For the convenience +of other checkers and the editor, the backwards checker should their +issues in forwards document order. See `.log.order`_. [This advice +may no longer be relevant with automated tools. RB 2023-01-26] + +_`.role.check.clarity`: The *clarity checking role* focuses on whether +the product document is clear and obvious. This is a good role to +give to someone who has never seen the product document before, but +who is in the intended readership. Anything that is unclear to them +is a defect. + +_`.role.check.consistency`: The *consistency checking role* focuses on +whether the product document or documents are internally consistent. + +_`.role.check.convention`: The *convention checking role* concentrates +on whether the product document complies with detailed conventions and +rules. + +_`.role.check.correctness`: The *correctness checking role* focuses on +whether the product document is correct, i.e. will have the intended +consequences. + +_`.role.check.source`: The *source checking role* concentrates on +whether the product document is consistent with any source documents, +and whether dependencies and links are documented where appropriate. + +[Other possible checking roles: + + - checking using a different medium (printouts) + - checking random things in a random order, using dice + - sampling large or repetitive changes at random + - build, test, lint, and other automated tools + +RB 2023-01-29] + + +9. Issue Classification +======================= + +[Imported from mminfo:guide.review.class and needs updating. RB +2023-01-26] + +_`.class`: There are many possible schemes for defect classification, +but only a coarse one is used here. Any issue raised, should fall into +one of the following classes. The normal abbreviation is indicated. + +_`.class.major`: (M): A Major defect is a defect in the Product +document that will probably have significantly increased costs to find +and fix later in the development process, for example in testing or in +use ([Gilb-93]_ p442). A bug that is fixed after review typically +takes one man-hour, after testing 10 man-hour, and in the field 100 +man-hours; and it's much worse for a garbage collector +(`.rationale.gc`_). A defect that will waste downstream development +effort is also major. Typical major defects are: + +- In an implementation, potentially failing to behave as specified; + +- In an implementation, failing to validate foreign data; + +- In a high-level document, being likely to cause major defects in + derived documents. + +_`.class.minor`: (m): A minor defect is any defect in the Product +document whose cost to fix does not increase in time. If there is a +typo, then it doesn't matter when it's fixed. Typical minor defects +are: + +- an implementation, poor variable names; + +- in any human-readable text, typos where the meaning is clear. + +_`.class.comment`: (C): A comment is any remark about the product +document. Typical comments are: + +- suggestions for how an algorithm could be optimised in future; + +- praise. + +_`.class.question`: (q): A question is any matter on which +`.role.checker`_ wants clarification. + +- If a product document is unclear to the intended readership then + that's also `.class.major`_ or `.class.minor`_, by + `rule.generic.clear`_. + +- Questions will be answered in writing (`.edit.act.answer`_). + Answering them often spawns changes anyway. + +- Typical questions are: + + - Clarifications on why things should be the way they are; + + - Curiosity about the details of something. + +_`.class.imp`: (I): An improvement suggestion is any potential defect +found in documents other than the product document. Typical +improvement suggestions are: + +- defects in source documents; + +- defects in rule sets, check lists, or procedures. + +_`.class.new`: (N): Any issue found during logging (as opposed to +during checking) is a new issue. This classification is orthogonal to +the preceding. It is important to mark new issues, in order to +measure how worthwhile group logging sessions are (see +`.log.purpose`_). + +.. _rule.generic.clear: rule.generic.rst#2 + + +11. Advice for the author +========================= + +_`.advice.author`: The intense scrutiny a formal review of your work +can be distressing. Remember that you are not under attack. Everyone +is working to make your work *better*. + +With that in mind, here is some advice from [Gilb-93]_: + + - Report your own noted issues after giving your team-mates a + chance. + + - Don't say 'I found that too!' + + - Thank your colleagues for their efforts on your behalf. + + - Learn as much as possible about avoiding the issues as an author. + + - Respect the opinion of your team-mates. Do not justify or defend. + + - Check the logging for legibility and intelligibility. + + - Answer any 'questions of intent' logged by checkers at the end of + the logging meeting. + + +12. Express review +================== + +_`.express`: The *express review procedure* [RB-2023-02-01]_ MAY be +executed by `.role.leader.experienced`_ to get a low-risk change +reviewed quickly, at low cost. + +_`.express.readership`: The readership of this section is +`.role.leader.experienced`_. + +_`.express.brief`: If anything in this section is unclear to you, +you're not ready to run express reviews. + +_`.express.try`: During an express review, if things go wrong, or turn +out to be riskier or more complicated than you thought, just go back +and `.plan`_ a full review. Record that you did. Don't delete the +express review record. + +_`.express.record`: Record the express procedure (`.doc.record`_). +You MAY squash the records for the other steps in one comment. + +_`.express.entry`: Execute `.entry`_ pretty much as usual. + +_`.express.call`: Call someone else in right now. + +_`.express.risk`: The other person MUST agree that the change has low +risk, and that express review will achieve the `.purpose`_. + +- Size is not risk. It's much more important to consider *what* is + being changed and *how*. + +_`.express.time`: Express review should take no more than about 30 +minutes. If it takes longer, revert to full review. + +_`.express.schedule`: No need to schedule. You both do it now. + +_`.express.train`: Choo choo! Don't do this with untrained people. +Revert to full review. + +_`.express.source`: All source docs MUST be immediately available. +If not, you know what to do by now. + +_`.express.rule`: Everyone SHOULD know the relevant rules. + +_`.express.homework`: If homework is needed, it's not an express +review. + +_`.express.remind`: Remind everyone of the `.purpose`_ of review. + +_`.express.role`: Everyone will perform every `.role.check`_. Not +feasible? It's not an express review. + +_`.express.improve`: Express reviews don't support extra objectives. + +_`.express.major`: If anyone finds `major defects`_, stop the express +review and `.plan`_ a full one. + +_`.express.save`: Save the review record before you check by +submitting the review record comment. GitHub has a tendency to lose +the draft if you start using your browser for checking. + +_`.express.check`: Do separate checking for some minutes. Look for +`major defects`_, note other issues. Don't confer. + +_`.express.log`: Confer. Announce issues, look for `major defects`_, +note other issues. + +_`.express.log.proper`: You still need to record issues properly, even +in an express review. Don't know how? You're not ready to run an +express review. + +_`.express.brainstorm`: Take a one minute break after logging then do +a few minutes of brainstorm. Prevention is still a goal. + +_`.express.edit`: If there are just a few minor edits, do them now, +together (like `pair programming`_). Otherwise, drop out of express +review into `.edit`_. Record this decision, natch. + +.. _pair programming: https://en.wikipedia.org/wiki/Pair_programming + +_`.express.pi`: Defer/delegate `.pi`_ but don't drop it. Prevention +is worth it. + +_`.express.exit`: Execute `.exit`_ pretty much as usual. Do record +metrics. + + +13. Rationale +============= + +_`.rationale`: Formal review is the key to the quality of the Memory +Pool System. + +A full justification of the review process described by this procedure +is not feasible here. There are three sources: + +1. the process improvement history of the Memory Pool System project, + +2. Software Inspection [Gilb-93]_, + +3. the analysis work behind the Capability Maturity Model + [CMU-SEI-93-TR-024]_. + +Of these, (1) is unfortunately the least accessible, because the +documents have travelled through several different systems, and +version control did not always survive. + +Ravenbrook does have hundreds of archived review records [MM-reviews]_ +with estimates of review productivity (produced by +`.phase.estimation`_). [At some point it would be good to summarize +those here. RB 2023-01-28] + + +13.1. Why formal reviews? +------------------------- + +Every formal review has been worthwhile in terms of preventing defects +versus the cost of review. + +The Harlequin MM Group adopted code review in the mid 1990s -- early +compared to most of the industry. Casual code reviews (where someone +eyeballs diffs) have become standard practice for many projects, and +it's quite hard to imagine a time without them. However, full-on +formal reviews or inspections are still relatively rare. + +_`.rationale.gc`: Formal review is appropriate for the MPS because +defects in memory managers, and especially in garbage collectors, are +*extremely* expensive to find and fix compared to other software. + +It's the job of a garbage collector to destroy information by +recycling (overwriting) objects and reorganizing memory. A subtle +failure of GC logic can cause a failure in the client software many +hours later. When that failure happens to a user of an application +delivered by developers using a compiler developed by your client that +uses the MPS in its runtime system, well, forget about it. A defect +in the compiler (usually considered expensive) is relatively cheap! + +This means that the cost of `major defects`_ escalates *much* more +steeply for the MPS than most software, so it is especially worthwhile +to catch them early in the development process. + +Even testing is too late. + + +A. References +============= + +.. [CMU-SEI-93-TR-024] "Capability Maturity Model for Software, + Version 1.1"; Mark C. Paulk, Bill Curtis, Mary + Beth Chrissis, Charles V. Weber; Software + Engineering Institute, Carnegie Mellon + University; 1993-02; + . + +.. [CMU-SEI-93-TR-025] "Key Practices of the Capability Maturity + Model, Version 1.1"; Mark C. Paulk, + Charles V. Weber, Suzanne M. Garcia, Mary Beth + Chrissis, Marilyn Bush; Software Engineering + Institute, Carnegie Mellon University; 1993-02; + . + +.. [Gilb-93] "Software Inspection"; Tom Gilb, Dorothy Graham; Addison + Wesley; 1993; ISBN 0-201-63181-4; book.gilb93. + +.. [MM-guide.review.edit] "Guidelines for review edits"; Gavin + Matthews; Harlequin Limited; 1996-10-31; + mminfo:guide.review.edit; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/guide/review/edit/index.txt#1. + +.. [MM-process.review] "The review process"; Richard Brooksby; + Harlequin Limited; 1995-08-18; + mminfo:process.review; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/process/review/index.txt#1. + +.. [MM-proc.review.brainstorm] "Procedure for process brainstorm in + review"; Gavin Matthews; Harlequin + Limited; 1997-06-12; + mminfo:proc.review.brainstorm; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/brainstorm/index.txt#1. + +.. [MM-proc.review.check] "Procedure for checking in review"; Gavin + Matthews; Harlequin Limited; 1997-06-12; + mminfo:proc.review.check; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/check/index.txt#1. + +.. [MM-proc.review.entry] "Procedure for review entry"; Gavin + Matthews; Harlequin Limited; 1997-06-02; mminfo:proc.review.entry; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/entry/index.txt#1. + +.. [MM-proc.review.exit] "Procedure for exiting a document from + review"; Gavin Matthews; Harlequin Limited; + 1997-06-12; mminfo:proc.review.exit; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/exit/index.txt#1. + +.. [MM-proc.review.ko] "Procedure for a review kickoff meeting"; Gavin + Matthews; Harlequin Limited; 1997-06-12; + mminfo:proc.review.ko; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/ko/index.txt#1. + +.. [MM-proc.review.log] "Procedure for review logging meeting"; Gavin + Matthews; Harlequin Limited; 1997-06-12; + mminfo:proc.review.log; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/proc/review/log/index.txt#1 + +.. [MM-reviews] Review records of the MM Group; Harlequin Limited; + mminfo:review.*; + //info.ravenbrook.com/project/mps/doc/2002-06-18/obsolete-mminfo/mminfo/review/... + +.. [RB-2023-02-01] "Express review notes and test"; Richard Brooksby; + Ravenbrook Limited; 2023-02-01; + . + +.. [RFC-2119] "Key words for use in RFCs to Indicate Requirement + Levels"; S. Bradner; Harvard University; 1997-03; + + (with errata). + + +B. Document History +=================== + +========== ===== ================================================== +2023-01-19 RB_ Created. +2023-01-20 RB_ Importing material from MM Group proc.review. +2023-01-26 RB_ Importing checking roles and issue classification + from MM Group documents. +2023-01-28 RB_ Developing the Rationale. + Tidying up remaining comments. + Revising entry, planning, kickoff, and exit. + Revising documents section. +2023-01-30 RB_ Revising checking, logging, and brainstorm. +2023-01-31 RB_ Revised based on `review test run`_. +2023-02-01 RB_ Implementing `.express`_. +2023-02-06 RB_ Checking against and referencing [Gilb-93]_. +2023-11-09 RB_ Updating prior to reviewing the review to fill in + holes based on recent experience. +========== ===== ================================================== + +.. _RB: mailto:rb@ravenbrook.com + +.. _review test run: https://github.com/Ravenbrook/mps/pull/123#issuecomment-1408682681 + + +C. Copyright and License +======================== + +Copyright © 2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/rule.code.python.rst b/procedure/rule.code.python.rst new file mode 100644 index 0000000000..2453cfed4c --- /dev/null +++ b/procedure/rule.code.python.rst @@ -0,0 +1,260 @@ +=============================== +Rules for source code in Python +=============================== + +:Tag: rule.code.python +:Author: Gareth Rees +:Organization: Ravenbrook Limited +:Date: 2001-05-23 +:Readership: Python programmers +:Confidentiality: public +:Copyright: See `C. Copyright and License`_ + + +1. Introduction +=============== + +This document is a ruleset which can be applied to Python source code. +It is intended for use with an inspection procedure [Gilb-1995]_ but +can also be used for general guidance and review. + +The intended readership is Ravenbrook staff, but these rules can used by +anyone. + +This document is not confidential. + + +2. Rules +======== + +_`.compatible`: Write code that's compatible with Python version 1.5.2 +and all later versions. [We should update this to Python 3. RB +2023-01-26] + +_`.global`: Use global variables only for read-only data. + +_`.instance`: Assign modifiable instance variables in a class's +``__init__`` method, not in the class definition at top level. + +_`.joining`: Don't use a backslash to join two lines unless you have +to. + +_`.unittest`: Use the Python unit test framework for your tests. + + +3. Justification and commentary +=============================== + +3.1. Compatibility +------------------ + +_`.compatible.just`: + +It's very annoying to have to upgrade your Python +installation in order to run a program. Requiring the newest version +of Python could be a barrier to a customer (who already has a Python +installation) using a program. + +Of course, this may not be possible, because you require a feature +that's only available in a newer version. If so, document this clearly +in the introduction to the module, package or product. + +Main points to note: + +#. Don't rely on nested environments (introduced in Python 2.1 + [Kuchling-2001a]_ `§2 + `__). + Pass in the environment you need as arguments to the locally + declared function or the ``lambda`` expression. + +#. Don't use the augmented assignment operators ``+=``, ``-=``, ``*=`` + and so on (introduced in Python 2.0 [Kuchling-2000]_ `§6 + `__). + Write ``a = a + b`` instead of ``a += b``. + +#. Don't rely on garbage collection of cycles (introduced in Python + 2.0 [Kuchling-2000]_, `§8 + `__). + Break the cycle explicitly. + +#. Be careful with the division operator on integers; in Pythons before + 3.0 it returns the floor of the result; from Python 3.0 ``x/y`` will + mean ``float(x)/float(y)`` (see [Kuchling-2001b]_ `§6 + `__). + Use ``int(math.floor(x/y))`` if you want the old meaning, or + ``float(x)/float(y)`` if you want the new meaning. (In a few cases it + doesn't matter.) + + +3.2. Globals +------------ + +_`.global.just`: + +Python's scope rules ([Python-2000-10-16]_, `§4.1 +`__) +mean that a mention of variable in a function definition may refer +either to a local variable or a global variable, depending on whether +the variable is assigned to anywhere in the function definition, +unless you remember to specify ``global variable``. This is a source +of hard-to-discover defects. + +Instead of using global variables, make classes and turn your global +variable into instance variables (but don't forget `.instance`_). This +also has the advantage of making it possible for several instances of +your code to run at the same time, thus meeting +`rule.code.adaptable`_. + +.. _rule.code.adaptable: rule.code.rst#2-rules + + +3.3. Instances +-------------- + +_`.instance.just`: + +Python's object instantiation rules ([Python-2000-10-16]_, `§7.6 +`__) +mean that variables declared at top level in a class are shared +between all instances of the class. That's OK for non-modifiable +variables (for example, ``i = 1``) because an assignment to a class +variable actually creates an instance variable that shadows the class +variable. But for lists, dictionaries and other modifiable objects you +may be surprised when you find out that your list is shared between +all instances of the class. + + +3.4. Joining +------------ + +_`.joining.just`: + +Backslashes don't cope with code reformatting. You can usually arrange +for lines to be join implicitly by adding extra parentheses. + +Sometimes this can't be done, for example the ``raise`` and ``print`` +statements sometimes need backslashes. But consider putting the long +expression in a variable. + + +3.5. Unit testing +----------------- + +_`.unittest.just`: + +The Python unit test framework [PyUnit]_ is the standard way of +developing sets of unit tests. (It is distributed with Python 2.1, but +needs to be downloaded and installed if you have an earlier Python. + +Using PyUnit makes it easier to collect sets of tests into test suites +and to pick out individual tests and run them. + + +A. References +============= + +.. [GDR-2001-05-25] + "Rules for source code style"; + Gareth Rees; + `Ravenbrook Limited`_; + 2001-05-25. + +.. [Gilb-1988] + "Principles of Software Engineering Management"; + Tom Gilb; + `Addison-Wesley`_; + 1988; + ISBN 0-201-19246-2. + +.. [Gilb-1995] + "Software Inspection"; + Tom Gilb, Dorothy Graham; + `Addison-Wesley`_; + 1995; + ISBN 0-201-63181-4. + +.. [Kuchling-2000] + "What's New in Python 2.0"; + `A M Kuchling `__ and + `Moshe Zadka `_; + 2000; + . + +.. [Kuchling-2001a] + "What's New in Python 2.1"; + `A M Kuchling `__; + 2001; + . + +.. [Kuchling-2001b] + "What's New in Python 2.2"; + `A M Kuchling `__; + 2001; + . + +.. [PyUnit] + "PyUnit - a unit testing framework for Python"; + `Steve Purcell `_; + . + +.. [Python-2000-10-16] + "Python Reference Manual (Python 2.0)"; + `Guido van Rossum `_; + 2000-10-16; + . + +.. [RB-2001-05-24] + "Re: Rules for Python"; + `Richard Brooksby `_; + `Ravenbrook Limited`_; + 2001-05-24; + . + +.. _`Addison-Wesley`: http://www.awl.com/ + + +B. Document History +=================== + +========== ===== ================================================== +2001-05-23 GDR_ Created based on outcome of informal inspection. +2001-05-25 GDR_ Moved style rule to [GDR-2001-05-25]_, following + the advice in [RB-2001-05-24]_. +2001-08-15 GDR_ Added warning about compatibility of division + operator between Python versions. +2023-01-26 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _GDR: mailto:gdr@ravenbrook.com +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +======================== + +Copyright © 2001-2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/rule.code.rst b/procedure/rule.code.rst new file mode 100644 index 0000000000..afd8f43d71 --- /dev/null +++ b/procedure/rule.code.rst @@ -0,0 +1,316 @@ +===================== +Rules for source code +===================== + +:Tag: rule.code +:Author: Richard Brooksby +:Organization: Ravenbrook Limited +:Date: 1998-06-30 +:Readership: any programmer +:Confidentiality: public +:Copyright: See `C. Copyright and License`_ + + +1. Introduction +=============== + +This document is a ruleset which can be applied to any source code. It +is intended for use with an inspection procedure [Gilb-1995]_ but can +also be used for general guidance and review. + + +2. Rules +======== + +_`.adaptable`: Non-adaptable code may not be used where there is a more +adaptable alternative. + +_`.assume`: Code may not make assumptions about the inner workings of other +modules. + +_`.branch-edits`: Edits on a version or development branch must be easy to +merge. + +_`.break`: There must be documented justification wherever rules are broken +to meet requirements. + +_`.constraint`: Code constraints must be easy to modify. + +_`.deps`: Dependencies must be marked and cross-referenced at both ends. + +_`.design`: The code must not be the sole documentation of the design. + +_`.independent`: Platform dependent code may not be used where there is a +platform independent alternative. + +_`.justified`: The justification for the code must be clear or documented. + +_`.limits`: All limitations of a body of code (such as a module) must be +exposed to clients of that code (in its interface). + +_`.minimal`: The code must limit its functionality to what is required. +Extra functionality which arises from generality must be disabled. + +_`.req`: The code must meet its requirements. + +_`.simple`: The code must be as simple as possible (to meet requirements). + +_`.style`: Code must be written according to the relevant style guide. + +_`.tricky`: The code must not use clever tricks. Write straightforward code. + +_`.width`: Code should be no wider than 72 characters. + + +3. Justification and commentary +=============================== + +_`.justification-adaptable`: [Nothing written yet. Note that +“adaptability” is a measure of the cost of meeting new and changing +requirements. RB 1998-07-01] + +_`.justification-assume`: [Nothing written yet. RB 1998-07-01] + +_`.justification-branch-edits`: Edits on version branches will be +merged back to the master sources. Edits on development branches will be +merged back to the version branch or the master sources that they came +from. It must be possible to do the merges reliably and without +introducing errors. The goal is to make it possible to consider each +change separately, to decide which to merge, and for different people to +maintain the code along the version branch successfully. The fix on the +version branch may be a workaround and a better fix may need to be +developed separately. + +To do so, follow the following rules: + +1. Don't delete stuff. It will be hard to restore it. Comment it out + or skip it. + +2. Don't fiddle with the formatting of code or comments. It creates + bogus conflicts that create extra work when merging. + +3. Add comments explaining why a change was made (especially in version + branches). Sign the comments with your name and the date. Explain why + you had to make the change. Refer to defect reports when fixing them. + +Check in each defect fix separately. + +Write change comments that summarize what you did, and the quality of +the change (is it a permanent fix, a temporary workaround or what). Are +there any consequences that other people should know about? There's no +need to say which job you fixed: the fix will make that clear. + +The rule was introduced to fix job000070_. + +.. _job000070: https://info.ravenbrook.com/project/p4dti/issue/job000070 + +_`.justification-break`: It's sometimes necessary to break the rules to +meet the requirements. For example, there might be an urgent change, or +you might need to introduce a dependency that could be avoided. This is +OK, provided that the transgression is justified, which is to say, there +must be a convincing explanation of why you needed to break the rule. + +_`.justification-constraint`: A constraint is something which limits the +generality of the code in some way. The most common form of constraint +is a constant which bounds the size of something, for example, the +length of a fixed size array. This rule means that you must make it easy +to modify the size of the array, perhaps by defining the length as a +constant and using that everywhere. (See also rule.generic.once_, +which backs this up.) + +.. _rule.generic.once: /rule/generic#.once + +_`.justification-deps`: If there is a dependency between two pieces of +code, especially if they are a long way apart, the dependency must be +marked at both ends, in such a way as the other end of the dependency +can be found. It must be marked at the dependee end because a change +there will break the dependent code. It must be marked at the dependent +end so that the dependency can be understood when modifying that code. + +Changes in the presence of dependencies cause defects. They are one of +the things that only the code “wizards” understand, so that only they +can modify the code successfully. + +_`.justification-design`: [Nothing written yet. RB 1998-07-01] + +_`.justification-independent`: This rule is saying that you must write +platform independent code wherever you possibly can. The most common +argument for platform dependent code is that it has better performance. +Maybe. Platform-optimized code is only allowable if it has a documented +justification in terms of requirements (see the break rule). + +_`.justification-justified`: This rule is saying that it must be clear +why the code is like it is. So, if an obscure or counter-intuitive +structure is used, it must be explained. + +This rule makes the code maintainable, which is to say, adaptable at low +cost and without introducing defects. This rule ensures that the code +can be understood. + +_`.justification-limits`: [Nothing written yet. RB 1998-07-01] + +_`.justification-minimal`: The minimal rule avoids software entropy +and atrophy caused by Hyrum's Law [Hyrum-2020]_ : + + With a sufficient number of users of an API, + it does not matter what you promise in the contract: + all observable behaviors of your system + will be depended on by somebody. + +Enforcing minimal functionality enforces abstractions and prevents +dependency on unintended behaviour. + +_`.justification-req`: This is a restatement of the generic/achieve +rule. The difference here is that the purpose of code is more often +stated directly in terms of customer requirements, and the code should +refer to and be consistent with those. + +_`.justification-simple`: Write the simplest code you can. Don't +confuse simplicity with elegance. Don't try to be clever, try to be +defect free. You will find this hard enough without making things +complicated. + +_`.justification-style`: Almost any consistent style is better than a +mixture of styles, even if those styles are in some sense better. A +common style should be used to make the code easy for people to read. +Code which is easy to read is easier to check and more likely to be +correct. + +Ravenbrook's style guide is [GDR-2001-05-25]_. + +_`.jusitification-tricky`: This is a variation of the simple rule, +designed to catch those cases where a tricky piece of code is argued by +the author to be “simpler” than a longer but more straightforward piece +of code. For example:: + + if (a = call(b)) { + error("call failed"); + } + +It might be argued that this is simpler than:: + + a = call(b); + if (a != RESULT_OK) { + error("call failed"); + } + +But the former is a "trick" of the C language and is therefore not +allowed. + +Tricks of this sort are sources of defects because they are easily +misunderstood, and therefore will be modified incorrectly. + +_`.jusitification-width`: Many terminals, editors and mail user agents +have a standard width of 80 characters. If code is too wide, then it +will be hard to read and edit using these tools and that will mean that +defects are introduced. In particular, we want to support: + +- Editing in vi with line numbers turned on. + +- Editing in BBEdit without having to resize the window each time you + open a file. + +- Quoting source code in e-mail (possibly to two or three levels of + quoting) without causing it to wrap and become unreadable. + +See [RB-2001-05-15]_. + + +A. References +============= + +.. [GDR-2001-05-25] + "Rules for source code style"; + `Gareth Rees`_; + `Ravenbrook Limited`_; + 2001-05-25; + . + +.. [Gilb-1995] + "Software Inspection"; + Tom Gilb, Dorothy Graham; + Addison-Wesley_; + 1995; + ISBN 0-201-63181-4. + +.. [Hyrum-2020] + "Software engineering at Google: lessons learned from programming over time"; + Titus Winters, Tom Manshreck, Hyrum Wright, eds. (2020).; + O'Reilly Media, 2020; + ISBN 9781492082798; + . + +.. [RB-1998-06-30a] + "General Code Ruleset"; + `Richard Brooksby`_; + `Ravenbrook Limited`_; + 1998-06-30; + . + +.. [RB-1998-06-30b] + "Rules for all documents"; + `Richard Brooksby`_; + `Ravenbrook Limited`_; + 1998-06-30; + . + +.. [RB-2001-05-15] + "Re: Code width" (e-mail message); + `Richard Brooksby`_; + `Ravenbrook Limited`_; + 2001-05-15; + . + +.. _`Addison-Wesley`: http://www.awl.com/ +.. _`Richard Brooksby`: mailto:rb@ravenbrook.com +.. _`Gareth Rees`: mailto:gdr@ravenbrook.com + + +B. Document History +=================== + +========== ===== ================================================== +2001-04-22 GDR_ Created based on [RB-1998-06-30a]_. +2001-05-15 GDR_ Added width rule, based on [RB-2001-05-15]_. +2001-05-19 GDR_ Added branch-edits rule, based on RB's analysis in + job000070_. +2001-05-25 GDR_ Added reference to style guide [GDR-2001-05-25]_ + to style rule. +2015-12-16 RB_ Converted to ReStructuredText and released under + Creative Commons license. +2023-01-26 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _GDR: mailto:gdr@ravenbrook.com +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +======================== + +Copyright © 1998-2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/rule.code.style.rst b/procedure/rule.code.style.rst new file mode 100644 index 0000000000..567312ffce --- /dev/null +++ b/procedure/rule.code.style.rst @@ -0,0 +1,145 @@ +=========================== +Rules for source code style +=========================== + +:Tag: rule.code.style +:Author: Gareth Rees +:Organization: Ravenbrook Limited +:Date: 2001-05-25 +:Readership: programmers +:Confidentiality: public +:Copyright: See `C. Copyright and License`_ + + +1. Introduction +=============== + +This document is a style guide for source code. It is intended for use +with an inspection procedure [Gilb-1995]_ but can also be used for +general guidance and review. + +This style guide collects rules to do with code layout and style; we +have these rules so that documents are consistent with themselves +(`rule.generic.self`_) and each other +(`rule.generic.other`_). However, these details don't often cause +defects, so they are collected here to avoid other rule sets becoming +cluttered with unimportant details [RB-2001-05-24]_. + +.. _rule.generic.self: rule.generic.rst#2-rules +.. _rule.generic.other: rule.generic.rst#2-rules + +The intended readership is Ravenbrook staff, but these rules can used by +anyone. + +This document is not confidential. + + +2. General style rules +====================== + +_`.layout`: When editing someone else's document, follow the layout +conventions of that document (`rule.generic.self`_). + + +3. Rules for C-like languages +============================= + +These rules apply to language with C-like syntax (for example, C, C++, +Java, Perl, Python). + +_`.binop`: Put a single space either side of a binary operator (except +`.comma`_). + +_`.block`: When a block is delimited by braces, put the opening brace +at the end of the line introducing the block, closing brace on a line +by itself, at same intendation level as the line introducing the +block. + +_`.colon`: No space before a colon, one space after. + +_`.comma`: No space to the left of a comma, one space to the right. + +_`.comment`: Align comments with the code they refer to. + +_`.control`: One space between a control structure (``if``, ``while``, +``for``) and the opening parenthesis of its arguments. + +_`.function`: No space between a function and the opening parenthesis +of its arguments. + + +A. References +============= + +.. [GDR-2001-05-23a] + "Rules for source code in C-like languages"; + Gareth Rees; + `Ravenbrook Limited`_; + 2001-05-23. + +.. [GDR-2001-05-23b] + "Rules for source code in Python"; + Gareth Rees; + `Ravenbrook Limited`_; + 2001-05-23. + +.. [Gilb-1995] + “Software Inspection”; + Tom Gilb, Dorothy Graham; + Addison-Wesley_; + 1995; + ISBN 0-201-63181-4. + +.. [RB-2001-05-24] + "Re: Rules for Python" (e-mail message); + Richard Brooksby; + `Ravenbrook Limited`_; + 2001-05-24; + . + +.. _`Addison-Wesley`: http://www.awl.com/ + + +B. Document History +=================== + +========== ===== ================================================== +2001-04-22 GDR_ Created based on rules originally in + [GDR-2001-05-23a]_ and [GDR 2001-05-23b]_, + following the advice in [RB-2001-05-24]_. +2023-01-26 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _GDR: mailto:gdr@ravenbrook.com +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +======================== + +Copyright © 1998-2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/rule.generic.rst b/procedure/rule.generic.rst new file mode 100644 index 0000000000..66d1ab64dd --- /dev/null +++ b/procedure/rule.generic.rst @@ -0,0 +1,374 @@ +======================= +Rules for all documents +======================= + +:Tag: rule.generic +:Author: Richard Brooksby +:Organization: Ravenbrook Limited +:Date: 1998-06-30 +:Readership: anyone +:Confidentiality: public +:Copyright: See `C. Copyright and License`_ + + +1. Introduction +=============== + +This document is a set of rules which apply to any document. The term +document covers almost any work product, including source code. The +rules are intended for use with an inspection procedure [Gilb-1995]_ but +can also be used for general guidance and review. + + +2. Rules +======== + +_`.achieve`: A document must achieve (be consistent with) its purpose. + +_`.automated`: It should be possible to process a document using automated tools. + +_`.change`: It must be possible to retrieve a complete and justified history of changes to a document. + +_`.clear`: A document must be clear to the intended readership. + +_`.complete`: A document must be complete for its purpose. + +_`.confidential`: A document must state whether it is confidential, and if +so, who can read it. + +_`.ident`: Documents must be marked with a unique identifier which includes +the document revision, preferably a source control ID. + +_`.note`: Notes or annotations which do not form part of the main document +must be clearly indicated. + +_`.once`: A document must make statements once and thereafter refer to the +original statement. + +_`.other`: Documents must be consistent with other documents. + +_`.purpose`: The purpose of a document must be clear to the intended readership. + +_`.readership`: The intended readership of a document must be clear to anyone. + +_`.ref`: All statements in a document must be easily referenceable. + +_`.ref-contents`: Each entry in the references section (conventionally +appendix A) should consist of title, author, company, publisher, date, +ISBN, and URL (or as many of these as are appropriate to the reference). + +_`.ref-name`: A reference should be named after its author and date (if known). + +_`.ref-sort`: References should be sorted in order by name. + +_`.self`: Documents must be consistent with themselves. + +_`.simple`: A document must be as simple as possible for its purpose. +Simplicity is not the same as terseness. + +_`.sources`: All statements shall refer to sources which justify them. + +_`.standard`: A document must follow applicable standards. + + +3. Justification and commentary +=============================== + +This ruleset is designed to tackle the main causes of defects: + +1. poor communication, + +2. poor justification, + +3. inconsistency, + +4. incomplete or lost information. + +_`.justification-achieve`: Anything which does not achieve its purpose is +defective by definition. This rule allows one to express this simple +fact by reference to a rule. + +_`.justification-automated`: This means that documents should be +structured and formatted in a consistent and predictable way (which is +important for other reasons; see the `.self`_ and `.other`_ rules). + +There are many reasons why documents may need to be processed by +automated tools. For example: checking documents for errors in +formatting; checking the contents of one document against another it is +supposed to be consistent with; finding all documents that reference a +document (when the latter changes in some way); re-writing links when +part of a project is branched; editing using regular expressions or +editor macros. Not all reasons can be anticipated in advance. + +Automated checking and processing of documents is much more reliable +than doing the same by hand and is likely to result in fewer errors. + +_`.justification-change`: The purpose of the `.change`_ rule is to make +sure that the revision histories of all documents can be understood. +This is especially important with source code, where tiny changes can +critically affect product functionality, but is also important for other +documents. The word “justified” is important here. The *reason* for +each change must be clear as well as the change itself. + +If the intended readership does not have access to the source control +system, there must be a document history section (conventionally +appendix B). Each entry in this section should consist of the date the +document was changed, the initials of the person who changed it, and a +short statement of what was changed. + +If the intended readership have access to the source control system +containing the document, it is sufficient to give a source control +identifier for the document: see the `.ident`_ rule. However, it is a +good idea to have a document history section anyway, because the +readership may be expanded later, and it is easier to maintain a +document history from the beginning than to fabricate one later based on +the source control history. + +_`.justification-clear`: The purpose of a document is communication, and +if it is not clear to its intended readership then it is not achieving +that purpose. It is essential that the communication be effective, or +misunderstandings will happen and defects will result. This is perhaps +the most common source of defects. + +**Note**: If you are in the intended readership of a document and it is +not clear to you then it is *not clear* and it is breaking this rule. +You must not allow yourself to be persuaded (especially verbally) that +it is clear after all. + +_`.justification-complete`: An incomplete document may be adequate to get +basic ideas across, but “the devil is in the details”. Defects often +arise because of incomplete information. + +Incomplete source code (failing to cover all cases, for example) is +usually defective. + +_`.justification-confidential`: Ravenbrook staff work on a mixture of +confidential and publicly available documents. It is important not to +confuse the former with the latter, otherwise private material could be +accidentally be made available to the public. + +_`.justification-ident`: The `.ident`_ rule allows found documents to be +understood out of context. + +Putting a document under source control and assigning it a source +control ID (usually using the “keyword expansion” feature of the source +control system) automatically achieves the `.ident`_ rule but and also +most of the `.change`_ rule. The information system ensures that source +control IDs are easily translated to URLs which allow the document to be +retrieved from the information server. + +_`.justification-note`: Notes allow authors to add incidental information +to documents. This is often useful, but it is important to separate the +incidental information from the main part of the document so that it can +be understood to be incidental. + +[My recommended practice for notes, by the way, is to use square +brackets and to sign your name and put the date at the end. RB +1998-06-30] + +**Important**: Do not confuse this kind of “note” with source code +“comments” (for which “comment” is a misnomer). A source code comment +is a part of the document and must obey all the normal document rules. + +_`.justification-once`: Documents which contain redundancy are fragile: +it is easy to make them inconsistent when changed, introducing defects +which are hard to track down. Redundancy should be avoided for this +reason, and any redundancy or dependency must be made very clear by +cross-referencing. + +_`.justification-other`: This is a very powerful rule when combined with +the `.sources`_ rule. Since every statement must be backed up by +sources, this rule allows one to check that the statement is in fact +consistent with those sources, and justified by them. Thus the +connection between customer needs, requirements, specification, changes, +and product is checked step-by-step. + +_`.justification-purpose`: If the purpose of a document is not clear then +it is not possible to check whether the document achieves its purpose +(see the `.achieve`_ rule). + +Note that this rule does not require the purpose to be explicitly +stated, but it must be clear to the entire readership. Usually it +should be stated. + +_`.justification-readership`: The main purpose of this rule is to support +the `.clear`_ rule. Without it, “clarity” cannot be defined. + +The other purpose of this rule is to help people deal with “found” +documents. Since anyone can identify the readership they know who to go +to for an interpretation of the document. + +_`.justification-ref`: Statements must be easily referenced to support +cross-referencing from other documents (see the `.sources`_ rule) and +therefore checking for consistency between documents (see the `.other`_ +rule). Inconsistency between separate documents is a major source of +defects. + +Similarly, statements must be easily referenced to support the `.once`_ +rule, since self-inconsistency is another important source of defects. + +_`.justification-ref-contents`: This is a specialization of the +`.sources`_ rule. + +_`.justification-ref-name`: Dates must be in standard format [ISO-8601]_. +Use as much of the date as you know. + +For authors who are Ravenbrook staff, use their initials, for example, +[RB-1998-06-30], for consistency with Ravenbrook convention in e-mail +and messaging (`.other`_). For other authors, use the surname, for +example [Gilb-1995], for consistency with general convention. +Distinguish documents written by the same author on the same date with +letters after the date, for example [RB-1998-06-30a]. Where you don’t +know the actual author, you can use the company, for example +[Perforce-2001-04-13], or make up a descriptive reference, for example +[XHTML-1.0]. + +_`.justification-ref-sort`: Sorting the references by name makes it +possible to find the reference you’re looking for. + +_`.justification-self`: Self-inconsistency almost always indicates a +defect, because it indicates that the author (or authors) are not +communicating correctly. + +Inconsistency is also a needless source of complexity. If a document +does something one way, and then a similar thing a different way, then +it is not simple enough. + +_`.justification-simple`: + + “Everything should be made as simple as possible, but no simpler.” + (after Albert Einstein) + +Complexity is a source of defects. Something which is complex is hard +to understand, and therefore we can be less sure that it meets its +requirements. The quality of complex things is therefore almost +inevitably lower than that of simple things. + +Simple documents are easier to understand, maintain, and adapt. +Simplicity therefore reduces cost as well as increasing quality. + +Software is complex enough without making it any more complex. Our +customer’s requirements are also complex and contradictory. We must +therefore combat complexity at every turn, or it will overwhelm us and +we will lose. + +_`.justification-sources`: The main purpose of this rule, combined with +the `.other`_ rule, is to ensure that decisions are justified in terms +of customer needs. This improves quality by directing all decisions +towards customer need. + +The secondary (but still very important) purpose of this rule is to make +it possible to understand the document in the future when we have +forgotten its connections to other documents. This makes it possible to +maintain and adapt the document, and also makes it possible to detect +when the document is out of date with respect to other changes (another +big source of defects). + +The source documents of source code are often issue or change documents +which caused that code to be the way it is. + +The sources for a document should be listed in a references section. + + +_`.justification-standard`: Following applicable standards helps a +document to follow the `.self`_, `.other`_, and `.automated`_ rules. + +In particular: + +1. Dates and times should follow [ISO-8601]_. Write “2001-02-03”, not +“03/02/01”: the latter also means 1901-03-02, 2001-03-02, and other +dates. Write “09:45”, not “9:45”: the latter also means 21:45 (when +“pm” is understood). + +2. Currencies should follow [ISO-4217]_. Write “GBP”, not “£”: the +latter isn’t represented in ASCII (and so may be transmitted incorrectly +in e-mail). Write “USD”, not “$”: the latter is used for many +currencies, for example CAD, HKD, AUD, and NZD. + + +A. References +============= + +.. [Gilb-1988] + “Principles of Software Engineering Management”; + Tom Gilb; + Addison-Wesley_; + 1988; + ISBN 0-201-19246-2. + +.. [Gilb-1995] + “Software Inspection”; + Tom Gilb, Dorothy Graham; + Addison-Wesley_; + 1995; + ISBN 0-201-63181-4. + +.. [ISO-4217] + “ISO 4217:1995 Codes for the representation of currencies and funds”; + ISO_; + 1995. + +.. [ISO-8601] + “ISO 8601:2000 Data elements and interchange formats -- Information + interchange -- Representation of dates and times”; + ISO_; + 1988-06-15. + +.. [RB-1998-06-30] + “Generic Ruleset”; + `Richard Brooksby`_; + `Ravenbrook Limited`_; + 1998-06-30. + +.. _`Addison-Wesley`: http://www.awl.com/ +.. _`ISO`: http://www.iso.ch/ +.. _`Richard Brooksby`: mailto:rb@ravenbrook.com + + +B. Document History +=================== + +========== ===== ================================================== +2001-04-22 GDR_ Created based on [RB-1998-06-30]_. +2001-06-07 GDR_ Added `.standard`_ rule. +2001-07-11 GDR_ Moved rules ref-contents, ref-name and ref-sort + from XHTML ruleset because they apply to all + documents, not just XHTML documents. +2015-12-15 RB_ Converted to ReStructuredText and released under + Creative Commons license. +2023-01-26 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _GDR: mailto:gdr@ravenbrook.com +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +======================== + +Copyright © 1998-2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end diff --git a/procedure/rule.style.rst b/procedure/rule.style.rst new file mode 100644 index 0000000000..8ec0dc2afd --- /dev/null +++ b/procedure/rule.style.rst @@ -0,0 +1,136 @@ +======================= +Rules for writing style +======================= + +:Tag: rule.style +:Author: Gareth Rees +:Organization: Ravenbrook Limited +:Date: 2001-12-02 +:Readership: anyone +:Confidentiality: public +:Copyright: See `C. Copyright and License`_ + + +1. Introduction +=============== + +This document is a set of rules which apply to manuals and other +technical documentation aimed at users of software. The rules are +intended for use with an inspection procedure [Gilb-1995]_ but can +also be used for general guidance and review. + +These rules were originally stated in [Corman-2001-11-27]_. + +The intended readership is Ravenbrook staff, but these rules can used by +anyone. + +This document is not confidential. + + +2. Rules +======== + +_`.explicit`: Give explicit instructions on how to carry out a task. + +_`.indicative`: Use the indicative mood. + +_`.lists`: Use numbered lists when sequence is important, unnumbered +otherwise. + +_`.may`: Be careful with "may". + +_`.task`: Organize user documentation by task. + +_`.present`: Use the present tense where possible. + + +3. Justification and commentary +=============================== + +_`.explicit.just`: Don't just state the goal of the task and leave the +reader to figure out how to do to it: some readers might not know what +to do, or might decide to do the wrong thing. + +_`.indicative.just`: Avoid the subjunctive mood (clauses introduced +with "could", "should", or "would"). This leaves the reader uncertain +as to whether to do the task or not. It's OK to use "should" as a +technical term conveying a level of requirement, if you've defined it +as such. + +_`.lists.just`: [I don't know the justification for this -- GDR +2001-12-02] + +_`.may.just`: The word "may" is ambiguous; it means both "be allowed +to" and "possibly". Use "might" for the second sense. It's OK to use +"may" as a technical term conveying a level of requirement, if you've +defined it as such. + +_`.task.just`: Organization by task makes it easy for the reader to +find the section corresponding to the task they want to perform, and +makes it more likely that they'll carry out the task correctly. + +_`.tense.just`: [I don't know the justification for this -- GDR +2001-12-02] + + +A. References +============= + +.. [Corman-2001-11-27] + "General comments on DTI Advanced Admin Guide" (e-mail message); + Tony Corman; + Perforce Software; + 2001-11-27; + . + +.. [Gilb-1995] + “Software Inspection”; + Tom Gilb, Dorothy Graham; + Addison-Wesley_; + 1995; + ISBN 0-201-63181-4. + +.. _`Addison-Wesley`: http://www.awl.com/ + + +B. Document History +=================== + +========== ===== ================================================== +2001-12-02 GDR_ Created based on [Corman-2001-11-27]_. +2023-01-26 RB_ Integrated to MPS Git and prepared for public use. +========== ===== ================================================== + +.. _GDR: mailto:gdr@ravenbrook.com +.. _RB: mailto:rb@ravenbrook.com + + +C. Copyright and License +======================== + +Copyright © 2001-2023 `Ravenbrook Limited `_. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +1. Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +.. end