brion(a)svn.wikimedia.org schreef:
> Revision: 40736
> Author: brion
> Date: 2008-09-12 00:16:41 +0000 (Fri, 12 Sep 2008)
>
> Log Message:
> -----------
> some first steps towards my horrifically evil code review tool to integrate in the wiki (doesn't work yet :)
>
Sounds like a very interesting project.
> <snip>
>
> + -- Update type: Modify, Add, Delete, Rename
> + cp_action enum ('M','A','D','R'),
>
Does rename even exist in SVN? Correct me if I'm wrong, but I always
thought SVN implemented renaming as a combination of copy and delete.
Speaking about copy, do you have a way to store and display stuff like
(Copied from foo) for added files?
> +-- And for our commenting system...
> +-- To specify follow-up relationships...
> +CREATE TABLE /*$wgDBprefix*/code_relations (
> + cf_repo_id int not null,
> + -- -> cr_id
> + cf_from int not null,
> + -- -> cr_id
> + cf_to int not null,
> +
> + primary key (cf_repo_id, cf_from, cf_to)
> +) /*$wgDBTableOptions*/;
>
Why didn't you just add cr_prev and cr_next to code_revisions? Also, the
cf_ prefix is kind of a weird choice here, since there's no 'f' in
code_relations.
> +
> +-- Freetext tagging for revisions
> +CREATE TABLE /*$wgDBprefix*/code_tags (
> + ct_repo_id int not null,
> + ct_rev_id int not null,
> + ct_tag varbinary(255) not null,
> +
> + primary key (ct_repo_id,ct_rev_id,ct_tag),
> + key (ct_repo_id,ct_tag,ct_rev_id)
> +) /*$wgDBTableOptions*/;
> +
>
What exactly are these tags going to do? Native support for marking as
reviewed or reverted would be nice. Also, the only indices on this table
are (ct_repo_id, ct_rev_id, ct_tag) and (ct_repo_id, ct_tag, ct_rev_id).
Wouldn't (ct_tag, ct_repo_id, ct_rev_id) be useful too (e.g. for finding
all revisions in all repos with a certain tag)?
> +CREATE TABLE /*$wgDBprefix*/code_comment (
> + -- Unique ID of the comment within the system.
> + cc_id int auto_increment not null,
> +
> + -- Repo and code revision this comment is attached to
> + cc_repo_id int not null,
> + cc_rev_id int not null,
> +
> + -- Wikitext blob of the comment.
> + -- FIXME: Consider using standard text store?
> + cc_text blob,
>
Does 'standard' text store mean the text table or the external disk
thingy? I would recommend using the same class that stores revision
text, so this extension doesn't have to worry about whether external
storage is enabled or not.
> + -- Timestamps of threaded parent and self to present a
> + -- convenient threaded sort order:
> + -- "20080130123456"
> + -- "20080130123456,20080230123456"
> + -- "20080430123456"
> + --
> + -- Allows 17 levels of nesting before we hit the length limit.
> + -- Could redo more compactly to get 31 or 63 levels.
> + cc_sortkey varbinary(255),
>
Nice one. What will happen when the limit is hit, though?
> +
> + -- Does this comment confer a review sum?
> + -- 0, +1, -1
> + cc_review int,
> +
> + primary key (cc_id),
> + key (cc_repo_id,cc_rev_id,cc_sortkey)
> +) /*$wgDBTableOptions*/;
>
You might wanna add a cr_review field holding the sum of all these
cc_review's to the code_rev table, along with an index. With that, the
review sum will always be correct even if not all comments are
displayed, and it'll be possible to search for e.g. all revisions with a
negative review sum.
About the extension as a whole: are new revisions added to the system
the second they're made, or are they imported on a regular basis? Also,
it would be nice to have some kind of e-mail notification feature
(either through the wiki's e-mail system or through a mailing list).
Roan Kattouw (Catrope)