r95429 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95428‎ | r95429 | r95430 >
Date:19:53, 24 August 2011
Author:hashar
Status:reverted (Comments)
Tags:needs-php-test 
Comment:
add ability to attach a comment to a diff line

cc_patch_line === NULL means the comment is below the diff just like it
was before. Set to a number, it refers to the line number in the diff
file from 1 to n.

Still need an UI which is a work in progress.

NOTE: adding a simple field required a lot of files to be changed :(
Modified paths:
  • /trunk/extensions/CodeReview/CodeReview.php (modified) (history)
  • /trunk/extensions/CodeReview/api/ApiQueryCodeComments.php (modified) (history)
  • /trunk/extensions/CodeReview/api/ApiRevisionUpdate.php (modified) (history)
  • /trunk/extensions/CodeReview/archives/code_comment_patch_line.sql (added) (history)
  • /trunk/extensions/CodeReview/backend/CodeComment.php (modified) (history)
  • /trunk/extensions/CodeReview/backend/CodeRevision.php (modified) (history)
  • /trunk/extensions/CodeReview/codereview.sql (modified) (history)
  • /trunk/extensions/CodeReview/ui/CodeCommentsListView.php (modified) (history)
  • /trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CodeReview/CodeReview.php
——@@ -288,6 +288,8 @@
289289
290290 $updater->addExtensionUpdate( array( 'addIndex', 'code_rev', 'cr_repo_status_author',
291291 "$base/archives/code_revs_status_author-index.sql", true ) );
 292+ $updater->addExtensionUpdate( array( 'addField', 'code_comment', 'cc_patch_line',
 293+ "$base/archives/code_comment_patch_line.sql", true ) );
292294 break;
293295 case 'sqlite':
294296 $updater->addExtensionUpdate( array( 'addTable', 'code_rev', "$base/codereview.sql", true ) );
——@@ -298,6 +300,8 @@
299301 "$base/archives/code_signoffs_timestamp_struck.sql", true ) );
300302 $updater->addExtensionUpdate( array( 'addIndex', 'code_paths', 'repo_path',
301303 "$base/archives/codereview-repopath.sql", true ) );
 304+ $updater->addExtensionUpdate( array( 'addField', 'code_comment', 'cc_patch_line',
 305+ "$base/archives/code_comment_patch_line.sql", true ) );
302306 break;
303307 case 'postgres':
304308 // TODO
Index: trunk/extensions/CodeReview/backend/CodeRevision.php
——@@ -607,15 +607,16 @@
608608 * @param $text
609609 * @param $review
610610 * @param null $parent
 611+ * @param int $patchLine (default: null)
611612 * @return int
612613 */
613 - public function saveComment( $text, $review, $parent = null ) {
 614+ public function saveComment( $text, $review, $parent = null, $patchLine = null ) {
614615 $text = rtrim( $text );
615616 if ( !strlen( $text ) ) {
616617 return 0;
617618 }
618619 $dbw = wfGetDB( DB_MASTER );
619 - $data = $this->commentData( $text, $review, $parent );
 620+ $data = $this->commentData( $text, $review, $parent, $patchLine );
620621
621622 $dbw->begin();
622623 $data['cc_id'] = $dbw->nextSequenceValue( 'code_comment_cc_id' );
——@@ -685,9 +686,10 @@
686687 * @param $text
687688 * @param $review
688689 * @param null $parent
 690+ * @param int $patchLine (default: null)
689691 * @return array
690692 */
691 - protected function commentData( $text, $review, $parent = null ) {
 693+ protected function commentData( $text, $review, $parent = null, $patchLine = null ) {
692694 global $wgUser;
693695 $dbw = wfGetDB( DB_MASTER );
694696 $ts = wfTimestamp( TS_MW );
——@@ -697,6 +699,7 @@
698700 'cc_rev_id' => $this->id,
699701 'cc_text' => $text,
700702 'cc_parent' => $parent,
 703+ 'cc_patch_line' => $patchLine,
701704 'cc_user' => $wgUser->getId(),
702705 'cc_user_text' => $wgUser->getName(),
703706 'cc_timestamp' => $dbw->timestamp( $ts ),
——@@ -731,9 +734,17 @@
732735 }
733736
734737 /**
 738+ * @param $attached boolean Fetch comment attached to a line of code (default: false)
735739 * @return array
736740 */
737 - public function getComments() {
 741+ public function getComments( $attached = false ) {
 742+ $conditions = array(
 743+ 'cc_repo_id' => $this->repoId,
 744+ 'cc_rev_id' => $this->id );
 745+
 746+ if( $attached ) { $conditions['cc_patch_line!'] = null; }
 747+ else { $conditions['cc_patch_line'] = null; }
 748+
738749 $dbr = wfGetDB( DB_SLAVE );
739750 $result = $dbr->select( 'code_comment',
740751 array(
——@@ -741,12 +752,11 @@
742753 'cc_text',
743754 'cc_user',
744755 'cc_user_text',
 756+ 'cc_patch_line',
745757 'cc_timestamp',
746758 'cc_review',
747759 'cc_sortkey' ),
748 - array(
749 - 'cc_repo_id' => $this->repoId,
750 - 'cc_rev_id' => $this->id ),
 760+ $conditions,
751761 __METHOD__,
752762 array(
753763 'ORDER BY' => 'cc_sortkey' )
Index: trunk/extensions/CodeReview/backend/CodeComment.php
——@@ -1,7 +1,7 @@
22 <?php
33
44 class CodeComment {
5 - public $id, $text, $user, $userText, $timestamp, $review, $sortkey, $attrib, $removed, $added;
 5+ public $id, $text, $user, $userText, $timestamp, $review, $sortkey, $attrib, $removed, $added, $patchLine;
66
77 /**
88 * @var CodeRevision
——@@ -36,6 +36,7 @@
3737 $comment->user = $data['cc_user'];
3838 $comment->userText = $data['cc_user_text'];
3939 $comment->timestamp = wfTimestamp( TS_MW, $data['cc_timestamp'] );
 40+ $comment->patchLine = $data['cc_patch_line'];
4041 $comment->review = $data['cc_review'];
4142 $comment->sortkey = $data['cc_sortkey'];
4243 return $comment;
Index: trunk/extensions/CodeReview/codereview.sql
——@@ -173,6 +173,9 @@
174174 -- cc_id of parent comment if a threaded child, otherwise NULL
175175 cc_parent int,
176176
 177+ -- patch line the comment eventually applies to or NULL
 178+ cc_patch_line int default null,
 179+
177180 -- User id/name of the commenter
178181 cc_user int not null,
179182 cc_user_text varchar(255) not null,
Index: trunk/extensions/CodeReview/archives/code_comment_patch_line.sql
——@@ -0,0 +1,2 @@
 2+ALTER TABLE /*_*/code_comment
 3+ ADD COLUMN cc_patch_line int default null;
Index: trunk/extensions/CodeReview/api/ApiRevisionUpdate.php
——@@ -53,7 +53,10 @@
5454 $params['removeflags'],
5555 $params['addreferences'],
5656 $params['removereferences'],
57 - $params['comment']
 57+ $params['comment'],
 58+ null, // parent
 59+ 0, // review
 60+ $params['patchline']
5861 );
5962
6063 $r = array( 'result' => 'Success' );
——@@ -115,6 +118,10 @@
116119 ApiBase::PARAM_TYPE => 'integer',
117120 ApiBase::PARAM_ISMULTI => true,
118121 ),
 122+ 'patchline' => array(
 123+ ApiBase::PARAM_TYPE => 'integer',
 124+ ApiBase::PARAM_MIN => 1,
 125+ ),
119126 );
120127 }
121128
——@@ -130,6 +137,7 @@
131138 'removeflags' => 'Code Signoff flags to strike from the revision by the current user',
132139 'addreferences' => 'Add references to this revision',
133140 'removereferences' => 'Remove references from this revision',
 141+ 'patchline' => 'Diff line to attach the comment to (optional)',
134142 );
135143 }
136144
Index: trunk/extensions/CodeReview/api/ApiQueryCodeComments.php
——@@ -95,6 +95,9 @@
9696 if ( isset( $this->props['text'] ) ) {
9797 ApiResult::setContent( $item, $row->cc_text );
9898 }
 99+ if ( isset( $this->props['patchline'] ) ) {
 100+ $item['patchline'] = $row->cc_patch_line;
 101+ }
99102 return $item;
100103 }
101104
——@@ -122,6 +125,7 @@
123126 'user',
124127 'status',
125128 'text',
 129+ 'patchline',
126130 'revid',
127131 'revision',
128132 ),
Index: trunk/extensions/CodeReview/ui/CodeCommentsListView.php
——@@ -57,6 +57,8 @@
5858 'cr_status' => wfMsg( 'code-field-status' ),
5959 'cr_message' => wfMsg( 'code-field-message' ),
6060 'cc_text' => wfMsg( 'code-field-text' ),
 61+ # patch line is only used for API call. No need for an i18n message
 62+ 'cc_patch_line' => null,
6163 );
6264 }
6365
Index: trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php
——@@ -58,11 +58,12 @@
5959 * @param string $commentText Comment to add to the revision
6060 * @param null|int $parent What the parent comment is (if a subcomment)
6161 * @param int $review (unused)
 62+ * @param int $patchLine Patch line number to which the comment will be attached (default: null).
6263 * @return int Comment ID if added, else 0
6364 */
6465 public function revisionUpdate( $status, $addTags, $removeTags, $addSignoffs, $strikeSignoffs,
6566 $addReferences, $removeReferences, $commentText,
66 - $parent = null, $review = 0 ) {
 67+ $parent = null, $review = 0, $patchLine = null) {
6768 if ( !$this->mRev ) {
6869 return false;
6970 }
——@@ -110,7 +111,7 @@
111112 $commentId = 0;
112113 if ( strlen( $commentText ) && $this->validPost( 'codereview-post-comment' ) ) {
113114 // $isPreview = $wgRequest->getCheck( 'wpPreview' );
114 - $commentId = $this->mRev->saveComment( $commentText, $review, $parent );
 115+ $commentId = $this->mRev->saveComment( $commentText, $review, $parent, $patchLine );
115116
116117 $commentAdded = ($commentId !== 0);
117118 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r108350Reverting inline commenting from CodeReview...johnduhart06:55, 8 January 2012

Comments

#Comment by Johnduhart (talk | contribs)   05:29, 8 January 2012 
+			# patch line is only used for API call. No need for an i18n message
+			'cc_patch_line' => null,

This adds a empty column to the comments page.

Status & tagging log