User:Dantman/CodeReviewSystem

Ideas for a new git based review system written from scratch.


 * Roan wants any backend written in C++ and the front end to be PHP.
 * Front end PHP code may reuse some of the classes we use in MediaWiki (WebRequest, PathRouter, etc...) this could become a framework. eg: We might make them {SomeFrameworkPrefix}WebRequest, etc... and then move the generic stuff there while making WebRequest, etc... in MediaWiki extend them.
 * We should use personal public remotes instead of push hacks and auth.
 * The review system controls it's own repo; No one has the capability to push to this.
 * The review system can be told to fetch changes from any number of public repos (ie: users' public repos)
 * Using web hooks (github supports them) and push hooks on a wm hosted repo system the review system will be automatically notified to fetch from a repo (Of course you can also tell it to manually fetch)
 * After it fetches commits the review system will show you commits that are not yet part of the repo or a review branch. You will then be able to convert a selection of them into a collection of commits for review and they will be put into a review branch. While manual is an option, we may code some conventions into the system which will trigger it to automatically create review branches and collections. Or automatically make suggestions or multiple choice options that the user will confirm to quickly create what is desired.
 * Sets of commits for review will be review branches instead of changesets.
 * Instead of using --amend on anything you'll make new commits and have the review system pull them into the review branch.
 * When the review branch is finally ready for the mainline it'll be merged in as-is. No squashing or rebasing creating false histories.
 * You may want to squash some of them. For instance when someone detects a typo in the comment added. There's no reason to keep that in the history.
 * In the event of a merge conflict you'll be told to make a commit merging master into the branch and handle the conflicts yourself. You'll then push that commit to your public repo and have the review system pull it into the review branch. After that it'll be merged into master.
 * To avoid branch name conflicts and also to avoid having people doing plain checkouts ending up pulling every review branch we'll use special refs for the review branches:
 * refs/review-heads/ - These will be the heads of review branches. The is the internal identifier that the review system will use for each review branch. This branch name is always unique. If you re-do the same review branch it'll come out with a new . When new commits are pulled into a review branch the review-head will update.
 * refs/review/ - These will be symbolic-refs (see git help symbolic-ref). Each one of these will point to a refs/review-heads/ . The is the human readable name given to a review branch. These are basically aliases for review-heads. If a review branch is abandoned (or if it's merged we may also let you re-use names) this name may disappear from the server and it may be re-used for a later review branch. (These may end up closer to topics; eg: We may end up making this name point to the latest in a series of review-heads such that say of 5 collections of commits to be reviewed the refs/review/* points to only the latest one, while all the earlier ones will have no refs/review/*)
 * If someone fixes something in a review branch in a way you don't like the review branch may be forked:
 * When you fork a review branch a new review-head is created.
 * We may create fork review aliases in the form refs/review/ / such that forking refs/review/fubar leads to creating refs/review/fubar/1.
 * A forked review branch will be linked to the previous review branch in a way that lets people easily find it. Notifications may also be made notifying that the review branch was forked. And the new review branch will copy or share some other things from the previous review branch. Such as people watching to the review branch.
 * If you checkout a review branch; reset so that you are on an earlier commit in the same review branch and create a new set of commits (or --amend an existing commit); Then push to your public repo; When the review system fetches these it may notice you based this off of an existing review branch but newer commits have been discarded and will automatically create a fork of that review branch for you.
 * Review branches may be split:
 * If the series of commits made into a review branch are too many and you wish to approve only a collection of them a review branch can be split.
 * In the review UI you can select a point at which to split the review branch and then submit.
 * The review system will create one ore more new review-heads and point the previous portions of the review branch to those. Tweaks will be made in the database to create new collections for review.
 * The end result shown in the system will be that one collection of commits for review will become two or more collections of commits.
 * These automatically will depend on each other as if you had pushed a set of commits creating a collection for review. And then added more commits pushing them and
 * Review branch collections may be combined:
 * If you accidentally split up or pushed a fix that ended up creating a new collection a collection of commits in a review branch can be merged with the collection of commits it is based on.
 * The review system will drop one of the review-heads a review-head and update things so everything is merged together.
 * We may base what one to drop based on how much data is attached. eg: If you push a collection then right away combine it then the newer review-head may be dropped since you really wanted to add things to the other; While if you push a collection, and then push another collection which gets comments from users. And then do a combine it'll drop the older review-head since the newer one is what everyone is using; (Or maybe we'll let you pick in the ui; or perhaps we'll create a brand new review-head for the new collection )
 * The dropped review head won't actually be deleted from the database.
 * It'll be kept around to redirect people from the previous commit head to the new one.
 * We could update the refs/review-heads/* so that both review heads point to the same sha. Or use a symbolic-ref to point the dropped one to the new one.
 * Comments will probably have some sort of connection tying them to a specific commit rather than a whole collection of commits for review. This may or may not be reflected in the ui. This will let us attempt to intelligently handle comments when we split or fork a commit. In some cases we may make some sort of phantom references to the comments in another collection of commits.
 * A collection of commits depending on other unreviewed collections of commits will still have a review button. Using it and confirming the warning it gives you will automatically review all the collections it depends on along with the collection.
 * A user will have profile of their own:
 * They will be able to control a list of emails to attribute to them like in gerrit.
 * The profile will allow them to control a list of their public repos which the review system should pull from.
 * They will have a watchlist they can control.
 * We should try and ensure that any diff comparison we have is at least as good as gerrit's is.
 * Commenting, giving a +1 -1, and verifying collections
 * Since things would be reviewed in terms of collections of commits the page should have an intuitive list of those commits. GitHub does this quite well https://github.com/dantman/mediawiki-core/compare/master...skinrewrite-private
 * We should look into seeing if there are any libraries that let us output listed graphs into web pages the way desktop git viewers do. Or perhaps write something. We'll probably need to use svg+js if we want it to be sane.
 * While we may include gitweb since it's standard we should look into other git viewers to use as the primary viewer. Especially if that viewer includes line graphs like desktop git guis do.
 * Notification system should be pluggable (email, irc, etc... will be implemented as separate classes plugging themselves in) and be smart in it's implementation:
 * When a collections is merged we don't need a separate notification for any comments or review notes made at the same time.
 * When mass-reviewing (eg: Using the option that reviews all dependent collections we only need one notification)
 * Authz/authn system should be pluggable. At the very least, we need a plugin for LDAP authentication/authorization.

Links

 * Very rough project start https://github.com/dantman/gareth