Add Package Review chapter to packaging guide

3 files changed, 709 insertions, 10 deletions

diff --git a/doc/cli.sh b/doc/cli.sh
index 96caa9a..5378d37 100755
--- a/doc/cli.sh
+++ b/doc/cli.sh
@@ -57,6 +57,7 @@ function gen () # <name>
 --link-regex '%b(#.+)?%../../build2/doc/build2-build-system-manual.xhtml$1%' \
 --link-regex '%bpkg(#.+)?%../../bpkg/doc/build2-package-manager-manual.xhtml$1%' \
 --link-regex '%bbot(#.+)?%../../bbot/doc/build2-build-bot-manual.xhtml$1%' \
+--link-regex '%brep(#.+)?%../../brep/doc/build2-repository-interface-manual.xhtml$1%' \
 --link-regex '%testscript(#.+)?%../../build2/doc/build2-testscript-manual.xhtml$1%' \
 --output-prefix build2-toolchain- "${@}" $n.cli
 
diff --git a/doc/packaging-initial-review-checklist.md b/doc/packaging-initial-review-checklist.md
index 01b2008..6625e5f 100644
--- a/doc/packaging-initial-review-checklist.md
+++ b/doc/packaging-initial-review-checklist.md
@@ -4,12 +4,10 @@
 
 - [ ] Repository in the [build2-packaging organization][build2-packaging] if third-party package.
 - [ ] Repository/project/package names consistent with upstream, [Debian][debian-pkgs] (see [repository name][rep-name], [package name][pkg-name]).
-- [ ] If library without `lib` prefix, no clashes with executable named (see [package name][pkg-name]).
+- [ ] If library without `lib` prefix, no clashes with executable names (see [package name][pkg-name]).
 - [ ] Uses git submodule and symlinks for upstream, submodule at correct release commit (see [upstream submodule][upstream-submodule], [upstream symlinks][upstream-symlink]).
 - [ ] Follows upstream layout (within reason) (see [package layout][upstream-layout]).
 - [ ] [Does not bundle dependencies][dont-bundle-deps].
-
-- [ ] Successful builds on [queue.cppget.org][queue].
 - [ ] Package archive sizes are not excessive and don't contain unnecessary files.
 
 - [ ] `manifest`: `name`/`project`/`version` values make sense.
diff --git a/doc/packaging.cli b/doc/packaging.cli
index 004e27e..71616d2 100644
--- a/doc/packaging.cli
+++ b/doc/packaging.cli
@@ -429,6 +429,10 @@ the repository root is the package root. See \l{bdep-new(1)} for details on
 how to initialize such a repository. In this guide, however, we will continue
 to assume a multi-package repository setup.|
 
+\N|Make sure the first commit in the package repository contains no manual
+changes. In other words, it should only add files as generated by
+\c{bdep-new}. This is relied upon during the package review process (see
+\l{#review-init-pr Create review pull request} for details).|
 
 \h2#core-repo-submodule|Add upstream repository as \c{git} submodule|
 
@@ -4082,14 +4086,15 @@ in the source distribution preparation (see \l{#core-test-smoke-locally-dist
 Test locally: distribution}).|
 
 If everything looks good, then you are done: the package submission will be
-reviewed and, if there are no problems, moved to \l{https://cppget.org
-cppget.org}. If there are problems, then an issue will be created in the
-package repository with the review feedback. In this case you will need to
+moved to \l{https://cppget.org cppget.org} for further testing and review. If
+this further testing or review identifies any problems with the package, then
+an issue will be created in the package repository with the feedback (see
+\l{#review Package Review} for details). In this case you may need to
 \l{#core-version-management-new-revision release and publish a version
-revision} to address any problems. However, in both cases, you should first
-read through the following \l{#core-version-management Package version
-management} section to understand the recommended \"version lifecycle\" of a
-third-party package.
+revision} to address any serious problems. But before doing that (or releasing
+a new version), you should first read through the following
+\l{#core-version-management Package version management} section to understand
+the recommended \"version lifecycle\" of a third-party package.
 
 Also, if there is an issue for this package in
 \l{https://github.com/build2-packaging/WISHLIST
@@ -4769,6 +4774,12 @@ difficult to guarantee that this requirement is satisfied. As a result, you
 should refrain from making major changes, like substantially altering the
 build, in a revision, instead delaying such changes until the next version.
 
+The recommendation is to limit revision changes to bug fixes and preferably
+only in the package \"infrastructure\" (\c{buildfiles}, \c{manifest},
+etc). Fixes to upstream source code should be limited to critical bugs and be
+preferably backported from upstream. To put it another way, changes in a
+revision should have an even more limited scope than a patch release.
+
 
 \h1#howto|Packaging HOWTO|
 
@@ -5502,4 +5513,693 @@ information will need to be updated manually. In this case feel free to submit
 a pull request with the necessary changes or
 \l{https://build2.org/community.xhtml#help get in touch}.
 
+
+\h1#review|Package Review|
+
+Due to the complexity of packaging C/C++ software and to maintain a standard
+of quality, package submissions must be reviewed by someone knowledgeable in
+\c{build2} and experienced in packaging third-party software before they can
+be deemed stable and moved from the \c{testing} to the \c{stable} section of
+the \l{https://cppget.org cppget.org} repository. This applies to initial
+package submissions, new versions, and new revisions. This chapter describes
+the review process.
+
+First, let's recap the package transition policies from
+\l{https://queue.cppget.org queue.cppget.org} to \l{https://cppget.org
+cppget.org} and between the various \l{https://cppget.org/?about sections of
+cppget.org}.
+
+All three types of submissions (initial, new version, and new revision) are
+performed with \l{bdep-publish(1)} and automatically placed into
+\l{https://queue.cppget.org queue.cppget.org} where they are built and tested
+as archive packages in the same set of build configuration as
+\l{https://cppget.org cppget.org}.
+
+\N|Publishing the package into the queue is the only automatic step and all
+other transitions are currently performed manually after review or evaluation
+by a \c{build2} core team member (though some steps may be automated in the
+future).|
+
+When the package is published to \l{https://queue.cppget.org
+queue.cppget.org}, it ends up in one of the \l{https://queue.cppget.org/?about
+three sections}: \c{alpha}, \c{beta}, or \c{testing}. The destination section
+is normally determined automatically by \l{bdep-publish(1)} based on the
+package version: alpha semver versions go to \c{alpha}, beta \- to \c{beta},
+and the rest (as well as non-semver versions) go to \c{testing}.
+
+If the package published to \l{https://queue.cppget.org queue.cppget.org}
+successfully builds in at least one build configuration, then it is migrated
+to \l{https://cppget.org cppget.org}. When the package is migrated from
+\l{https://queue.cppget.org queue.cppget.org}, it is placed into the
+corresponding section of \l{https://cppget.org cppget.org}, that is, a package
+from the \c{alpha} section of the former go to \c{alpha} of the latter,
+\c{beta} \- to \c{beta}, and \c{testing} \- to \c{testing}.
+
+If the package was migrated to either the \c{alpha} or \c{beta} section, then
+this is its final destination. If, however, the package was placed into the
+\c{testing} section, then it stays there for some time (at least a week) to
+allow further testing and review by any interested users. It is then migrated
+to the \c{stable} section provided the following conditions are met:
+
+\ol|
+
+\li|The package has at least one positive review that was performed by an
+experienced \c{build2} user.|
+
+\li|There are no serious issues reported with the package based on further
+testing.||
+
+If the package has no reviews, then it will remain in the \c{testing} section
+until reviewed, potentially indefinitely. Likewise, if the package has a
+negative review that identified blocking issues, then they must be addressed
+by the package author in a revision, published to queue, and re-reviewed.
+Until then the package will remain in \c{testing} but in severe cases (for
+example, security vulnerability and no forthcoming fix), it may also be
+dropped.
+
+\N|If the package has both positive and negative reviews, then the
+contradictions will be reconciled by the \c{build2} core team members.|
+
+\N|Packages in the \c{alpha} and \c{beta} sections can also be reviewed
+and a negative review may lead to the package being dropped in severe
+cases.|
+
+For completeness, let's also mention the \c{legacy} section of
+\l{https://cppget.org cppget.org}. A package that is no longer maintained
+(either by upstream or by the \c{build2} project) may be moved to \c{legacy}
+for two primary reasons: to signal to the users that it has serious issues
+(such as security vulnerabilities) and/or not to waste CI resources on
+building it.
+
+How are the transitions effected, exactly? Both \l{https://queue.cppget.org
+queue.cppget.org} and \l{https://cppget.org cppget.org} package repositories
+are backed by \c{git} repositories hosted at \l{https://github.com/cppget/
+github.com/cppget/}. Each transition described above (including the initial
+submission by \l{bdep-publish(1)}) is recorded as a commit in one or both of
+these repositories (study their commit histories to get a sense of how this
+works).
+
+While all the transitions can be performed manually by moving files around and
+using \c{git} directly, the \c{manage} script in
+\l{https://github.com/build2/bpkg-util \c{bpkg-util}} provides a high-level
+interface for the common transitions.
+
+Note also that the person doing a review and the person effecting a package
+transition need not be the same. For security, package transitions can
+currently only be performed by the \c{build2} core team members.
+
+With this background, let's now turn to the review process. At the outset you
+may be wondering why perform it so late in the packaging process when the
+final version has been released and published. Specifically, would it not be
+better to review some form of work-in-progress so that any issues can be
+corrected before releasing the final version?
+
+There are several reasons why we prefer to review the released version.
+Firstly, we want to base our reviews on the build results of the final package
+archives as they will be published to \l{https://cppget.org cppget.org}.
+Basing reviews on anything other than that means we will need to re-examine
+them when the final version is submitted.
+
+The second reason is consideration for the reviewer. Reviewing other people's
+packaging work, often for software you personally have no interest in using,
+is a thankless and often frustrating job (things tend to get frustrating when
+you have to point out the same basic mistakes over and over again). As a
+result, we want to make this job as streamlined as possible without
+(hopefully) multiple rounds of reviews. Also, the finality of the submission
+will hopefully encourage authors to try to submit finished work rather than
+something incomplete, for example, in the hope that the reviewer will help
+them fix it into shape.
+
+Having said that, nothing prevents members of the \c{build2} community from
+performing ad hoc pre-reviews, for example, for packaging efforts of new
+authors that require some help and guidance.
+
+\N|For background, this review process was heavily inspired by the Linux
+kernel development. Specifically, Linux developers review proposed changes and
+mark them with a number of tags, such as
+\l{https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
+\c{Reviewed-by}}. Below is the relevant quote from that document that gives a
+good description for what it means to offer a \c{Reviewed-by} tag and that
+matches many of the aspects of this review policy:
+
+\b{Reviewer’s statement of oversight}
+
+By offering my \c{Reviewed-by} tag, I state that:
+
+\ol|
+
+\li|I have carried out a technical review of this patch to evaluate its
+appropriateness and readiness for inclusion into the mainline kernel.|
+
+\li|Any problems, concerns, or questions relating to the patch have been
+communicated back to the submitter. I am satisfied with the submitter’s
+response to my comments.|
+
+\li|While there may be things that could be improved with this submission, I
+believe that it is, at this time, (1) a worthwhile modification to the kernel,
+and (2) free of known issues which would argue against its inclusion.|
+
+\li|While I have reviewed the patch and believe it to be sound, I do not
+(unless explicitly stated elsewhere) make any warranties or guarantees that it
+will achieve its stated purpose or function properly in any given situation.||
+
+A \c{Reviewed-by} tag is a statement of opinion that the patch is an
+appropriate modification of the kernel without any remaining serious technical
+issues. Any interested reviewer (who has done the work) can offer a
+\c{Reviewed-by} tag for a patch. This tag serves to give credit to reviewers
+and to inform maintainers of the degree of review which has been done on the
+patch. \c{Reviewed-by} tags, when supplied by reviewers known to understand
+the subject area and to perform thorough reviews, will normally increase the
+likelihood of your patch getting into the kernel.
+|
+
+Before we delve into the review process, let's also discuss how one finds
+packages to review. There are two common strategies: The first is to look at
+the packages that you are using in your own projects and, if there are any
+unreviewed versions that you would like to use, to consider reviewing them.
+Alternatively, if you would like to help the \c{build2} community by reviewing
+packages, pick any from the unreviewed list. For both cases you could use
+\l{https://cppget.org/?advanced-search Advanced Package Search}, limiting the
+result to specific packages in the first case. For example, you could search
+for
+\l{https://cppget.org/?advanced-search&rp=pkg%3Acppget.org%2Ftesting&rv=unreviewed
+unreviewed packages in the \c{testing} section}.
+
+The review process for the three types of submissions (initial, new version,
+and new revision) are described in the following sections.
+
+\N|At this stage of the \l{https://cppget.org cppget.org} repository
+evolution, where it primarily contains third-party packages, we are only
+concerned with reviewing the build and packaging support, ignoring everything
+else (source code, documentation, etc). This, however, may change in the
+future.|
+
+
+\h#review-init|Reviewing initial package submission|
+
+A review process for an initial package submission is the most elaborate since
+we need to review everything from scratch.
+
+
+\h2#review-init-issue|Create review issue|
+
+The first step in reviewing the initial package submission is to create a
+review issue on the package repository. The issue title should be in the
+following form (here and below \c{X.Y.Z} is the version being reviewed):
+
+\
+Review of the `X.Y.Z` version (initial package submission)
+\
+
+\N|Before actually creating the issue you may also want to check if someone
+else is already reviewing this package and thus has already created the issue.
+While there is nothing wrong with having multiple reviews, you may want to
+consider picking something else to review in order to increase coverage.|
+
+For issue description, copy and paste the contents of
+\l{https://raw.githubusercontent.com/build2/build2-toolchain/master/doc/packaging-initial-review-checklist.md
+packaging-initial-review-checklist.md}.
+
+Then create the issue.
+
+
+\h2#review-init-pr|Create review pull request|
+
+The only place where GitHub supports code reviews is a pull request (PR). As a
+result, we create a dummy draft pull request against the \c{master}/\c{main}
+branch whose only purpose is to serve as a code review vehicle. The
+procedure is as follows:
+
+\ol|
+
+\li|Clone the package repository (referred to as \c{<repo>}) locally:
+
+\
+git clone --recurse-submodules --shallow-submodules git@github.com:build2-packaging/<repo>.git
+cd <repo>
+\
+
+|
+
+\li|Find the first commit:
+
+\
+git rev-list --all | tail -1
+\
+
+Make sure it is the canonical \c{Initialize package repository} commit from
+the \l{#core-repo-init Initialize package repository with \c{bdep\ new}} step:
+
+\
+git log -p \"$(git rev-list --all | tail -1)\"
+\
+
+\N|The changes made in this commit will not be part of the review so we need
+to make sure nothing was lumped into it beside the project infrastructure
+created by \c{bdep-new}. We have to skip this commit because the two branches
+we will be creating the pull request from (see below) must have common
+history.|
+
+|
+
+\li|Create the review branch:
+
+\
+git branch review-X.Y.Z \"$(git rev-list --all | tail -1)\"
+git push origin review-X.Y.Z
+\
+
+|
+
+\li|Create pull request:
+
+  \ol|
+
+  \li|\nOpen the GitHub link printed by the above \c{git-push} command.|
+
+  \li|\nChange \c{base:} branch to \c{review-X.Y.Z}, \c{compare:} branch to
+  \c{master}/\c{main}.|
+
+  \li|\nFor PR title use:
+
+  \
+  Dummy draft pull request for version `X.Y.Z` review (do not merge)
+  \
+
+  |
+
+  \li|\nIn PR description link to the review issue (\c{<N>} is the review
+  issue number):
+
+  \
+  See review issue #<N>.
+  \
+
+  |
+
+  \li|\nChange the creation mode to \c{Create draft pull request} and create
+  the PR.||
+
+||
+
+\N|The review pull request is setup as if we wanted to merge the
+\c{master}/\c{main} branch into \c{review-X.Y.Z}, which generally doesn't make
+much sense (and is the reason why this PR should never be merged). However,
+this setup allows us to do code reviews of all the changes since the first
+commit. What's more, if the package author addresses some issues by releasing
+revisions, the revision commits will be automatically added to this PR
+and their changes available to further review.|
+
+
+\h2#review-init-checklist|Go through review checklist|
+
+Go through the review checklist in the review issue ticking off successfully
+completed items.
+
+While reviewing an item you may identify an issue or have something to note.
+A note is a general observation that is worth mentioning to the package
+author. For example, while checking this item:
+
+\
+[ ] If library without lib prefix, no clashes with executable names
+\
+
+You may note that while there are no clashes with executables installed in
+\c{PATH} locations, there is a package in Debian that has a private executable
+named like this. So you may make a note along these lines:
+
+\
+There is a file in Debian named ftest. It's not an executable installed
+in a PATH location so I guess having this library named without the
+lib prefix is technically allowed, though not ideal. Consider in the
+future to follow the recommendation in the packaging guide and name
+libraries with the lib prefix to sidestep such considerations.
+\
+
+An issue can be blocking or non-blocking. As the name suggests, a blocking
+issue fails the review and must be addressed in a revision. A non-blocking
+issue does not fail the review and can be optionally addressed in a revision
+or in the next version.
+
+Whether an issue is blocking or not is a judgment call of the reviewer (which
+is one of the reasons why a reviewer needs to be knowledgeable in \c{build2}
+and have experience packaging third-party projects). The overall guideline is
+to consider two dimensions: severity and impact. An issue that would prevent a
+large proportion of users from using the package is most likely
+blocking. Also, conceptual issues, like
+\l{https://github.com/build2/HOWTO/blob/master/entries/compile-options-in-buildfile.md
+using compile/link options that should not be specified in \c{buildfiles}},
+are always blocking. Finally, also consider whether it will be possible to fix
+the issue later without breaking backwards-compatibility. For example,
+renaming the package once it's published will be disruptive. If you are unsure
+whether an issue should be considered blocking, contact other reviewers to
+discuss.
+
+A note or an issue can be communicated to the package author in two ways: you
+can add it to the outcome comment for the review issue (created in the
+following step) or you can add it as a code review comment in the review
+PR.
+
+The first way is more appropriate for general notes (like the example above)
+and issues (like missing \c{README} file). While code review comments work
+best when the issue is with a specific code fragment that can be pointed to.
+
+To add code review comments, go to the review PR created in the previous step,
+select the \"Files changed\" tab, and start the code review. For each comment,
+specify whether it is a blocking issue, a non-blocking issue, or a note.
+
+
+\h2#review-init-outcome|Add review outcome comment|
+
+Once you are done with the checklist, add a comment to the review issue with
+the outcome to notify the package author.
+
+If the review was successful (no blocking issues), start the comment with the
+following paragraph (here \c{<AUTHOR>} is the package author's user name on
+GitHub):
+
+
+\
+@<AUTHOR> Thank you for your submission! This version has passed the
+review. Below you will find a list of non-blocking issues and notes
+that have been identified during review. You can address the issues
+with a revision if you wish or, alternatively, in the next version.
+\
+
+Adjust the last two sentences accordingly if there are no issues/notes.
+
+If, however, there are blocking issues, start it with the following:
+
+
+
+\
+@<AUTHOR> Thank you for your submission! Unfortunately there are
+blocking issues and this version has failed the review. The list
+of blocking issues is provided below. Please consider addressing
+them (as well as the non-blocking issues, if you wish) in a
+revision and publishing it to continue this review.
+\
+
+Adjust the last sentence accordingly if there are no non-blocking issues.
+
+Then continue the comment with a list of blocking issues, non-blocking issues,
+and finally notes (remove sections that have no items):
+
+\
+Blocking issues:
+
+...
+
+Non-blocking issues:
+
+...
+
+Notes:
+
+...
+
+\
+
+If one or more issues or notes are captured as code review comments, then add
+a link to the review PR. Otherwise, describe them in the comment. For example
+(here \c{<NUM>} is the review PR number):
+
+\
+Blocking issues:
+
+* A number of blocking issues described in the code review: #<NUM>
+
+Non-blocking issues:
+
+* A number of non-blocking issues described in the code review: #<NUM>
+
+Notes:
+
+* There is a file in Debian named ftest. It's not an executable installed
+  in a PATH location so I guess having this library named without the
+  lib prefix is technically allowed, though not ideal. Consider in the
+  future to follow the recommendation in the packaging guide and name
+  libraries with the lib prefix to sidestep such considerations.
+\
+
+
+\h2#review-init-success|Finish successful review|
+
+If the review is successful and once the outcome comment has been added, edit
+the issue description and remove the first sentence that says the review is in
+progress.
+
+Also close (note: not merge) the review PR and, if there are no non-blocking
+issues, the review issue.
+
+\N|If there are non-blocking issues, then we leave the review issue open as a
+reminder to the package author to address them in the next version.|
+
+Finally, send a notification email to \c{review@cppget.org} as described in
+\l{#review-init-email Send review notification email}.
+
+
+\h2#review-init-unsuccess|Continue with unsuccessful review|
+
+If the review is unsuccessful and once the outcome comment has been added,
+send a notification email to \c{review@cppget.org} as described in
+\l{#review-init-email Send review notification email}.
+
+Then wait for the package author to address blocking issues and publish a
+revision (which will be reflected in the review PR). Also watch out for any
+questions in the review issue or code review comments in the PR.
+
+Once the revision is published, re-review the relevant changes and confirm
+they address blocking issues. Check off any outstanding items in the review
+checklist and also note which non-blocking issues were addressed for the new
+outcome comment.
+
+Then continue from the \l{#review-init-outcome Add review outcome comment}
+step by adding a new outcome comment. If all the blocking issues were
+addressed and no new blocking issues were identified, then the outcome is
+successful. Otherwise, it is unsuccessful and another review round is
+required: wait for another revision/comments, re-review, add another
+outcome comment, etc.
+
+
+\h2#review-init-email|Send review notification email|
+
+In case of both successful and unsuccessful reviews, send an email to
+\c{review@cppget.org} in order to notify the \c{build2} core team about the
+outcome. The requirements for this email are as follows:
+
+\ul|
+
+\li|\nThe \c{From} field should contain your real name. Your review is a
+statement of oversight and without a real name it doesn't have much value.|
+
+\li|\nThe \c{Subject} filed should be in the following form:
+
+\
+Review <PROJECT> <VERSION>
+\
+
+Here \c{<PROJECT>} is the project name to which the reviewed package or
+packages belong and \c{<VERSION>} is their version. Note that all the
+packages in a multi-package project must be reviewed together.|
+
+\li|\nIn the body of the email include the following information:
+
+\ul|
+
+\li|List of packages reviewed.|
+
+\li|Review outcome: \c{pass} or \c{fail}|
+
+\li|Link to the outcome comment in the review issue.|||
+
+|
+
+For example:
+
+\
+From: John Doe <john@example.org>
+Subject: Review spdlog 1.14.1+2
+
+Packages reviewed:
+
+spdlog
+spdlog-tests
+spdlog-bench
+
+Review outcome: fail
+
+Outcome link:
+
+https://github.com/build2-packaging/spdlog/issues/1#issuecomment-123
+\
+
+\N|For a successful review of a new revision or version of an existing package
+that has no non-blocking issues or notes, there may be no review issue (see
+\l{#review-new-ver Reviewing new version submission} for details). In such
+cases the outcome link may be omitted.|
+
+\N|The review outcome is recorded in the package metadata that is stored in
+the backing \c{git} repository. See \l{brep#package-review-manifest Package
+Review Manifest} for details.|
+
+
+\h#review-new-ver|Reviewing new version submission|
+
+\N|The following discussion assumes that you have read through \l{#review-init
+Reviewing initial package submission}.|
+
+The extent of changes to the build and packaging support in a new version can
+range from no changes, to only minor changes, to a complete rewrite. As a
+result, the review procedure for a new version varies depending on the changes
+and broadly consists of the following three alternatives:
+
+\ol|
+
+\li|If there are no substantial changes and no issues (blocking or
+non-blocking), then we can skip creating the review issue and go straight to
+the notification email.|
+
+\li|If there are no substantial changes but there are issues (blocking or
+non-blocking), then we create the review issue but skip the checklist.
+Creating the review PR is also optional.|
+
+\li|If there are substantial changes then we should use the \l{#review-init
+Reviewing initial package submission} procedure to re-review the package from
+scratch with the checklist.||
+
+
+\h2#review-new-ver-changes|Determine the extent of changes|
+
+In order to select the appropriate review procedure we need to determine the
+extent of the changes in the new version compared to the previous version,
+which we will refer to as the \"base version\".
+
+\N|The previous version on which we are basing this review needs to be already
+reviewed. Failing that, reviewing the difference doesn't make much sense. If
+the immediately preceding version is not reviewed, you have two choices:
+either review it first or base your review on an earlier, already reviewed
+version. If its review is unsuccessful, then you will need to pay attention
+to the issues identified in the previous review.|
+
+The recommended next step is to get a sense of the changes by examining the
+difference between the base and the new versions. This can be done in several
+ways. You could clone the package repository locally and use your favorite
+\c{git} tool (\c{git-diff}, \c{gitk}, etc) to view the cumulative changes
+between the two release commits. Alternatively, you can use the GitHub
+commit comparison support to compare the two release tags:
+
+\
+https://github.com/build2-packaging/<project>/compare/v1.2.0...v1.3.0
+\
+
+\N|Because the source code for the package comes from a \c{git} submodule, the
+changes that we see will be conveniently limited to the build and packaging
+support plus the related documentation.|
+
+Study the changes and determine which review procedure is appropriate. While
+all the consideration described in \l{#review-init Reviewing initial package
+submission} (and its checklist) apply to new versions, additional attention
+must be paid to backwards compatibility. Unfortunately, it's not uncommon for
+inexperienced package authors to break backwards compatibility at the build
+level (for example, by renaming exported targets, configuration variables,
+etc) even though the upstream respects its backwards compatibility obligations
+as signaled by the version.
+
+The common case when reviewing a new version is no changes to the build and
+packaging support other than the version increment in \c{manifest}. If that's
+the case or you don't see any issues with other changes, then you can proceed
+directly to \l{#review-init-email Send review notification email}. In this
+case you can omit the outcome link.
+
+\N|Another common case that can use the same shortcut is when the upgrade to
+the new version was contributed as a PR by someone other than the package
+author. If such a PR was reviewed and merged by the package author, then this
+same review can also count as a package review and the package author can
+\l{#review-init-email Send review notification email} using the PR as the
+outcome link.|
+
+At the other, thankfully more rare, extreme you may find the package
+substantially changed or completely rewritten. This, for example, can happen
+in response to a major version release if upstream decides to re-architect
+their source code layout. But it can also be the result of more benign
+changes. For example, if upstream adds a dependency on a testing framework in
+its tests, then the \c{build2} package will need to split the tests into a
+separate package. In case of such substantial changes it is recommended to
+follow the \l{#review-init Reviewing initial package submission} procedure.
+
+With the two extremes covered, this leaves the case of some changes that have
+issues, blocking or non-blocking. In this case the next step is to create the
+review issue.
+
+
+\h2#review-new-ver-issue|Create review issue|
+
+If the changes in the new version have some issues, then we create the review
+issue on the package repository. The issue title should be in the following
+form (here and below \c{X.Y.Z} is the version being reviewed):
+
+\
+Review of the `X.Y.Z` version
+\
+
+\N|Before actually creating the issue you may also want to check if someone
+else is already reviewing this package and thus has already created the issue.
+While there is nothing wrong with having multiple reviews, you may want to
+consider picking something else to review in order to increase coverage.|
+
+Unlike the initial package submission, here we don't use the review checklist
+since most items won't apply (but you are welcome to refer to it if you
+like). Instead, you can put your feedback directly in the issue description,
+similar to \l{#review-init-outcome Add review outcome comment}.
+
+If you find it helpful, you can also create a review pull request similar to
+\l{#review-init-pr Create review pull request}. In this case use the base
+version's release commit as the starting commit for the review branch.
+
+
+\h2#review-new-ver-success|Finish successful review|
+
+If the review is successful (no blocking issues), close (note: not merge) the
+review PR if one was created and, if there are no non-blocking issues, the
+review issue.
+
+\N|If there are non-blocking issues, then we leave the review issue open as a
+reminder to the package author to address them in the next version.|
+
+Finally, send a notification email to \c{review@cppget.org} as described in
+\l{#review-init-email Send review notification email}.
+
+
+\h2#review-new-ver-unsuccess|Continue with unsuccessful review|
+
+If the review is unsuccessful, send a notification email to
+\c{review@cppget.org} as described in \l{#review-init-email Send review
+notification email}.
+
+Then wait for the package author to address blocking issues and publish a
+revision (which will be reflected in the review PR, if any). Also watch out
+for any questions in the review issue or code review comments in the PR.
+
+Once the revision is published, re-review the relevant changes and confirm
+they address blocking issues. Also note which non-blocking issues were
+addressed for the outcome comment.
+
+Then continue from the \l{#review-new-ver-issue Create review issue} step
+except this time adding your feedback as an outcome comment rather than in the
+issue description. If all the blocking issues were addressed and no new
+blocking issues were identified, then the outcome is successful. Otherwise, it
+is unsuccessful and another review round is required: wait for another
+revision/comments, re-review, add another outcome comment, etc.
+
+
+\h#review-new-rev|Reviewing new revision submission|
+
+The procedure for reviewing a new revision submission is essentially the same
+as \l{#review-new-ver Reviewing new version submission} (including sending the
+notification email). However, there are the additional requirement of the
+revision containing only minor changes and being strictly backwards-compatible
+with the version it replaces. See \l{#dont-big-changes-revision Don't make
+extensive changes in a revision} for background and details.
+
 "