> I've been wondering how to formalize patch acceptance at code freeze, but
> it might be a good idea to start talking about some way to maybe put
> brakes on patches earlier, ie some kind of "required approval process".
We went through this here for the bk-3.0 release. We're a much smaller
team so this may not work at all for you, but it was very successful
for us, so much so that we are looking at formalizing it in BK. But
you can apply the same process outside of BK just fine.
We created a well known spot for pending patches; all reviewers need access
to that spot. Here's the README from that directory:
There should be the following subdirectories here
ready/ -> waiting on review
done/ -> in the tree
rejected/ -> no good
In the ready/ subdirectory, for each repository which has changes that
want to be in bk-3.0 but are not, I want:
ready/atrev -> /home/bk/wscott/bk-3.0-atrev
ready/atrev.RTI
ready/atrev.REVIEWED
The first is a symlink to the location of the repository.
The second is an RTI request which describes what is in the repo and why
it should go in.
The third contains the review comments in the form
lm (approved|not approved)
review comments
wscott (approved|not approved)
review comments
etc.
Once the REVIEWED file contains enough approvals, in the judgement
of the gatekeeper, then he pulls the repo into the bk-3.0 tree and moves
the 3 files from ready/* to done/*
The things which worked very well were:
a) extremely simple. As we added developers they understood right away
what the process was.
b) centralized location. Anyone could be bored and go do a review.
c) tight control on the tree.
We're thinking about formalizing this in the context of BK as follows:
NAME
bk queue - manage the queue of pending changes
DESCRIPTION
bk queue is used to manage a queue of changes to a repository.
It is typically used on integration repositories where tighter
controls on change are desirable.
In all commands, if no URL is specified, the implied URL is the
parent of the current repository, if any. The URL "." means this
repository.
XXX - need a large paragraph on the importance of not circulating
changesets which are in review state. They'll come back.
bk queue [-n<name>] [-R<rti>] [<URL>]
This is like a bk push but wants a "request to integrate"
(RTI) which is sent with the changes. It also wants a name
for the set of changes. All pending changesets are pushed.
If no name is given, the user is prompted for one. If no
RTI is given, the user is prompted for one.
bk queue -l [-n<name>] [<URL>]
Lists the set of pending changes in the queue like so:
<name> <date> <user> <state>
Values for the <state> field:
unreviewed - nobody has looked at it yet
reviewed by <reviewer> on <date> - obvious
accepted - it is in accepted state but not integrated
rejected - reviewed and rejected
Note that if there are multiple reviewers of a change, there
will be multiple lines in the listing for that change.
If the <name> arg is present then restrict the listing to
that name. If the <name> arg is present more than once,
restrict the listing to the set of named changes.
Could also have a -s<state> option which restricts the listing
to those changes in <state> state.
bk queue -pR [-o<file>] <name> [<URL>]
Retrieves and displays the RTI for change <name>.
If <file> is specified, put the form there.
bk queue -pr [-o<file>] [-u<user>] <name> [<URL>]
Retrieves and displays the review form[s] for change <name>.
If a user is specified, retrieve that users' review only.
If <file> is specified, put the form there.
bk queue -uR [<rti>] [<URL>]
Replaces any existing RTI with the specified RTI. If no RTI
is specified, it prompts you for one like bk setup does.
bk queue -ur [<review>] [<URL>]
Adds or replaces any existing review form with the specified
review. If no review is specified, it prompts you for one
like bk setup does. You may only replace your own reviews.
bk queue -O[<owner>] [<URL>]
Sets the owner of the repository to <owner>. Only the owner
may update the repository. Only the current owner can change
the ownership. If no owner is specified and there is an owner
and the caller is the owner, then delete the owner.
(This is nothing more than a pre-{incoming,commit}-owner trigger)
bk queue -d<name> [-f] [<URL>]
Delete the named change from the queue. This deletes EVERYTHING,
the patch, rti, reviews, everything. Only the submitter of the
change may delete the change unless the -f option is supplied.
bk queue -U<name> [-R<rti>] [<URL>]
Replace the changes in the queue <name> with the set of
changesets in the current repository. If the <rti> is
present, replace the current RTI form with the specified form.
All reviews, if any, are updated with a note that indicates
the existing review was against changes which have been replaced.
GUI
This is a command line tool; Bryan gets to do bk queuetool
using these interfaces.
TODO
- how do we merge?
- define a format for the RTI
- define a format for reviews
- should the RTI & review files be KV files?
- should the {name/RTI/REVIEWS} live as part of the repo and be
propogated? I think yes for upstream propogation, no for
downstream. Hard to say.
- need a way to add a queue item with no changes, i.e., an RFE which
needs to be in the tree but there are no changes yet.
FILES
BitKeeper/queue/<name>/CSETS - changeset keys for change <name>
BitKeeper/queue/<name>/RTI - RTI for change <name>
BitKeeper/queue/<name>/PATCH - BK patch for change <name>
BitKeeper/queue/<name>/RESYNC - exploded patch for change <name>
BitKeeper/queue/<name>/review.user - review by user for change <name>
-- --- Larry McVoy lm at bitmover.com http://www.bitmover.com/lm - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon Dec 23 2002 - 22:00:20 EST