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.
 * It turns out to be more advantageous to write the system in python. Daniel Friesen (Dantman) (talk) 18:17, 27 March 2013 (UTC)
 * 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. ~Platonides))
 * ((You can amend and rebase yourself. Creating forks and having those merged. Just as we do now with pushing a new changeset. This is just noting that the system itself won't force any rebases in ways that rewrite the history and create potential conflicts and make working commits break after being committed. Daniel Friesen (Dantman) (talk) 00:29, 9 April 2012 (UTC)))
 * 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.
 * We probably want to layer this so we can support built-in password auth, taking accounts and passwords from an auth system like LDAP, and using a single SSO using an external page. As well as perhaps arbitrary 3rd party auths like Open ID, various OAuth sites, GitHub, etc...
 * Registration url should be arbitrarily override able.


 * Gareth should use a STOMP server
 * Using a PUBLISH/SUBSCRIBE model Gareth publishes a message on every new review, update, fetch, etc...
 * A set of code dedicated to WebSocket handling subscribes to a portion of these and relays them to clients that ask for specific ones (provided they have permission)
 * This would power the fetch progress bar, a live "Most recent review branches" list on the frontage, etc...
 * Internally Gareth would actually subscribe to the STOMP server inside of a thread/process dedicated to email notifications. Instead of doing it right away.
 * For with a digest preference it would actually queue things up and wait for a timer to finish. Allowing messages to be grouped together.
 * Similarly IRC notifications would be handled by a thread/process dedicated to IRC that has subscribed to certain types of STOMP messages.
 * https://code.google.com/p/stomppy/
 * http://stomp.github.com/implementations.html
 * https://activemq.apache.org/apollo/
 * https://code.google.com/p/coilmq/


 * Gareth should have an API:
 * eg: /api/1/projects
 * Project list, User list, Remotes list, etc...
 * Gareth should also have search & filter functionality:
 * Filters like project:mediawiki/extensions/* should work.
 * User filters should accept username, realname, and email. (Link these all to identity?)
 * Consider Lucene indexing commit messages, whole file contents (latest), and commit changes.
 * Search should have an option to switch between review, commit, project, etc... searching.
 * Search box should use contentEditable to create an intuitive interactive search input.
 * Autocomplete things like project:, owner:, as well as actual input.
 * Individual items like project:mediawiki/core should turn into whole bubbles.
 * Search functionality should be available via the API.
 * Urls like /p/mediawiki/extensions/* should work to view the project list page with a filtered list of projects.
 * We may want to have actual labelled groups for project repos to allow the projects page to have multiple sections of repos.


 * Support the Activity Streams and OStatus protocols alongside Atom (PubSub too?), etc... so not only can feed readers get live updates about new review sets, comments, etc... but one can also subscribe using things like Status.Net.


 * Creation of new project repos is done using the same kind of review sets as for commits.
 * Anyone can create one of these from a "Create project" button on the projects page.
 * They are attached to an anonymous repo some actions can be taken on.
 * A remote can still be attached to this like normal. So the create project request can either create an empty repo or simultaneously also be a review of some (or all) of the commits already in the repo.
 * Besides "Cancel" and "Submit for review" (or just "Submit"?) when creating a review set you have the rights to self review (especially for project creation) there's also a "Submit & Accept" button which will bypass the normal "Create review set then accept with the 'Reject' and 'Accept' buttons" pattern and result in immediate review and creation of the project.
 * Likewise any review set can be marked as belonging to any branch, including those that don't exist. So having the commit reviewed will result in the branch being created.
 * An ACL system handles permissions.
 * View, submit (for review), accept, self-merge, and create project.
 * Individual ACL rules can apply to everyone, specific users, and groups.
 * View and create project rights can only be filtered by project name.
 * Accept and self-merge rights are filtered by project and branch.
 * Accept rights also allow you to create any branch with a pattern you have the rights to accept something inside.
 * Filters for applying ACL rules can be made as strict comparisons, simple patterns like, anchored regular expressions.
 * Gareth will probably need storage for keys and logins of it's own to access 3rd party repos. Primarily to support private repos. Since a user's own public remote will have to require credentials to view. Support for hosting personal repos and forks may be necessary to make support for private repos have sane usability.
 * We should have a proper group system.
 * Internal groups can be edited and have specific users (or even other groups) belong to them.
 * Should also support smart groups as types that can be implemented by plugins. Such as an LdapGroup smart group implemented by the LDAP auth plugin. Which would take some options to define a pattern of LDAP users that belong to the group.


 * Starred projects (or some other term)
 * Various degrees of watch settings [Not watching (Notifications when reviewing), Digest (maybe not), Watching (notifications for all commits or all matching a pattern), Ignoring (no notifications, even if you're reviewing)]. For repos. And certain patterns in those repos (like modifications to certain files).
 * Reviewers reviewing other people's change sets could use something like an 'Archive' button that would leave them as a reviewer and retain the watchlist. But drop the change set off their dashboard until someone actually responds on it.
 * Dashboard (with filtered/per-project dashboard versions) with your change sets, ones you're reviewing, and helpful extra boxes like "Recently merged" and "Recently abandoned or rejected". As well as starred commits and projects.
 * Named tagging for commits.
 * Instead of -1 and +1 we should give each of the community two of each, like a (-2, -1, +1, +2) — though we might have reason to not include +2. The difference between -1 and 2 (we won't use those terms) essentially being "There's something wrong with the current state of the review branch" which only applies to the current state and requires re-review later vs. "I'm opposed to the fundamental idea of this change." which usually won't change no matter how many new commits are made.
 * Gerrit just kills off old review statuses. We have one more permanent version of -1. But we should also not completely kill non-permanenet reviews. Instead we should display them out in a greyed out format. To indicate it doesn't necessarily apply to the current state and the user needs to re-review. But that someone did in fact -1 (or +1) the code before.

Links

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

These things may have some use in the project:
 * GitLabs/GitHub style look could be a good starting point for themeing: http://gitlabhq.com/demo.html
 * CGit has an ASCII tree graph: http://hjemli.net/git/cgit/log/ We really want a graphical one though.
 * Visualization libraries may come in handy:
 * http://mbostock.github.com/d3/ex/
 * http://mbostock.github.com/protovis/ex/
 * http://prefuse.org/gallery/
 * http://thejit.org/demos/
 * http://thejit.org/static/v20/Jit/Examples/Spacetree/example1.html
 * http://thejit.org/static/v20/Jit/Examples/Hypertree/example1.html
 * There is a LDAP user provider for Symfony which may be useful later on https://github.com/opensky/LdapBundle