BMO/Splinter: Difference between revisions
< BMO
(Created page with "== q1/2-2012 splinter work == === phase one - address current splinter pain-points === bugs that should be easy(ish) to fix, to be addressed before the rewrite. ==== patch par...") |
|||
| (13 intermediate revisions by the same user not shown) | |||
| Line 1: | Line 1: | ||
== q1/2-2012 splinter work == | == q1/2-2012 splinter work == | ||
== phase one - address current splinter pain-points == | |||
bugs that should be easy(ish) to fix, to be addressed before the rewrite. | bugs that should be easy(ish) to fix, to be addressed before the rewrite. | ||
=== patch parsing missing features === | |||
add now | add now | ||
* {{bug|652334}} splinter doesn't support hg rename | * <strike>{{bug|652334}} splinter doesn't support hg rename</strike> | ||
* {{bug|695662}} splinter does not show bzr renames | * <strike>{{bug|695662}} splinter does not show bzr renames</strike> | ||
=== patch parsing bugs === | |||
only action quick fixes, more complex issues can wait for the PatchReader work | only action quick fixes, more complex issues can wait for the PatchReader work | ||
* {{bug|685645}} splinter diff viewer gets confused by some diffs | * <strike>{{bug|685645}} splinter diff viewer gets confused by some diffs</strike> | ||
* {{bug|709897}} splinter doesn't like hg diffs where a file was copied and then modified | * <strike>{{bug|709897}} splinter doesn't like hg diffs where a file was copied and then modified</strike> | ||
* {{bug|815531}} splinter fails to display attachment 681995 correctly | * <strike>{{bug|815531}} splinter fails to display attachment 681995 correctly</strike> | ||
* | * <strike>{{bug|826214}} New file with one line isn't shown</strike> | ||
=== bugs === | |||
investigate and fix if easy | investigate and fix if easy | ||
* {{bug|652500}} Splinter creates a comment with code review notes in the wrong place | * {{bug|652500}} Splinter creates a comment with code review notes in the wrong place | ||
* {{bug|797840}} Replying to a comment on Splinter always replies to the first comment | * <strike>{{bug|797840}} Replying to a comment on Splinter always replies to the first comment</strike> | ||
=== enhancements === | |||
fix now, all look easy | fix now, all look easy | ||
* {{bug|760989}} Splinter should not require login | * <strike>{{bug|760989}} Splinter should not require login</strike> | ||
* | * <strike>{{bug|821889}} Make it so that Splinter shouts loudly when a patch introduces Windows line endings</strike> | ||
fix if there's time | fix if there's time | ||
* {{bug|692431}} Splinter doesn't clear drafts when publishing a review | * {{bug|692431}} Splinter doesn't clear drafts when publishing a review | ||
* {{bug|823448}} Show exact replacement of code and not only the addition | |||
* {{bug|785314}} splinter's routines which add review links to email need to be updated for html bugmail | |||
== phase two - rewrite/replace splinter == | |||
=== mozilla-hosted reviewboard instance === | |||
=== | deploy http://www.reviewboard.org/ onto a mozilla infrastructure, and modify/extend it to integrate tightly with bugzilla (eg authentication). | ||
further investigation required, however this is now the preferred approach. | |||
=== based on webkit's review system instead of splinter === | |||
rewrite to become more like a normal bugzilla perl/js extension, rather than a pure-js one. | rewrite to become more like a normal bugzilla perl/js extension, rather than a pure-js one. | ||
this paves the way for | this paves the way for this system to replace upstream's diff view | ||
* https://bugs.webkit.org/attachment.cgi?id=3046&action=review | |||
* code @ http://trac.webkit.org/browser/trunk/Websites/bugs.webkit.org | |||
** simpler ui than splinter | |||
==== improvements over splinter ==== | |||
* simpler ui than splinter | |||
* simpler code than splinter | |||
* more features | |||
** comment context selection | ** comment context selection | ||
** side-by-side/unified views | ** side-by-side/unified views | ||
** preview | ** preview | ||
** copy without line numbers | ** copy without line numbers | ||
==== implementation | ==== conversion ==== | ||
* jquery --> yui3 conversion (http://www.jsrosettastone.com/ ) | |||
* rewriting the ruby diff parser in perl (hopefully using PatchReader) | |||
==== migration/implementation ==== | |||
* existing splinter comments should be understood or migrated | |||
* add a new longdescs fields (or use extradata if possible) to indicate that a comment is a review, rather than parsing comments | |||
==== add missing splinter features ==== | |||
* | * viewing one file at a time | ||
* | * marking a file as 'reviewed' | ||
* collapsing files | |||
* | |||
==== | ==== add new features ==== | ||
* interdiff | |||
==== other notes ==== | |||
* | * investigate licensing, especially with upstream bugzilla as the target | ||
* | * look at existing bugs filed against the system (eg https://bugs.webkit.org/show_bug.cgi?id=43948 ) | ||
Latest revision as of 06:38, 29 May 2013
q1/2-2012 splinter work
phase one - address current splinter pain-points
bugs that should be easy(ish) to fix, to be addressed before the rewrite.
patch parsing missing features
add now
bug 652334 splinter doesn't support hg renamebug 695662 splinter does not show bzr renames
patch parsing bugs
only action quick fixes, more complex issues can wait for the PatchReader work
bug 685645 splinter diff viewer gets confused by some diffsbug 709897 splinter doesn't like hg diffs where a file was copied and then modifiedbug 815531 splinter fails to display attachment 681995 correctlybug 826214 New file with one line isn't shown
bugs
investigate and fix if easy
- bug 652500 Splinter creates a comment with code review notes in the wrong place
bug 797840 Replying to a comment on Splinter always replies to the first comment
enhancements
fix now, all look easy
bug 760989 Splinter should not require loginbug 821889 Make it so that Splinter shouts loudly when a patch introduces Windows line endings
fix if there's time
- bug 692431 Splinter doesn't clear drafts when publishing a review
- bug 823448 Show exact replacement of code and not only the addition
- bug 785314 splinter's routines which add review links to email need to be updated for html bugmail
phase two - rewrite/replace splinter
mozilla-hosted reviewboard instance
deploy http://www.reviewboard.org/ onto a mozilla infrastructure, and modify/extend it to integrate tightly with bugzilla (eg authentication).
further investigation required, however this is now the preferred approach.
based on webkit's review system instead of splinter
rewrite to become more like a normal bugzilla perl/js extension, rather than a pure-js one. this paves the way for this system to replace upstream's diff view
- https://bugs.webkit.org/attachment.cgi?id=3046&action=review
- code @ http://trac.webkit.org/browser/trunk/Websites/bugs.webkit.org
improvements over splinter
- simpler ui than splinter
- simpler code than splinter
- more features
- comment context selection
- side-by-side/unified views
- preview
- copy without line numbers
conversion
- jquery --> yui3 conversion (http://www.jsrosettastone.com/ )
- rewriting the ruby diff parser in perl (hopefully using PatchReader)
migration/implementation
- existing splinter comments should be understood or migrated
- add a new longdescs fields (or use extradata if possible) to indicate that a comment is a review, rather than parsing comments
add missing splinter features
- viewing one file at a time
- marking a file as 'reviewed'
- collapsing files
add new features
- interdiff
other notes
- investigate licensing, especially with upstream bugzilla as the target
- look at existing bugs filed against the system (eg https://bugs.webkit.org/show_bug.cgi?id=43948 )