[webkit-dev] code review tool changes

[Adam Barth]

Thanks for the feedback.  Please feel encouraged to let me and others
know how you'd like these tools improved.

Adam


On Wed, Feb 9, 2011 at 10:49 AM, Oliver Hunt <oliver at apple.com> wrote:
> I just discovered that the review tool doesn't save the general comments,
> only inline ones (http://webkit.org/b/54121).  This just got me when I
> clicked on the style bot to see what style checks had failed and lost my
> feedback :(
> This was particularly annoying as i had command-clicked the link (to open in
> a new tab) and the link actually forced the current page to navigate instead
> - http://webkit.org/b/54113
> --Oliver
>
> On Feb 8, 2011, at 2:39 PM, Ojan Vafai wrote:
>
> On Wed, Feb 9, 2011 at 7:59 AM, Kenneth Russell <kbr at google.com> wrote:
>>
>> On Tue, Feb 1, 2011 at 6:02 PM, Kenneth Russell <kbr at google.com> wrote:
>> > On Tue, Feb 1, 2011 at 5:09 PM, Ojan Vafai <ojan at chromium.org> wrote:
>> >> There's been a slew of changes to the code review tool. It's probably a
>> >> good
>> >> time to send a summary now that I don't intend to add significant new
>> >> features.
>> >> -Side-by-side diffs: You can view the entire diff or individual files
>> >> in
>> >> side-by-side diff. If you change the entire diff, we'll store that in
>> >> localstorage and load diffs in side-by-side by default.
>> >> -Comments and diff navigation: n/p keys will navigate to the
>> >> next/previous
>> >> comment. j/k will navigate to the next/previous diff.
>> >> -Draft comments persist: draft comments are now stored in localstorage,
>> >> so
>> >> they will persist across reloads, crashes, etc. Since it's in
>> >> localstorage
>> >> it's stored per-machine.*
>> >
>> > Thank you for this in particular. Several times I've accidentally
>> > navigated away from a review and lost work.
>>
>> I just temporarily lost Internet connectivity while uploading a
>> (large) review and when I came back to the review (same browser, etc.)
>> all of my comments were lost. Is this functionality expected to work
>> with Chrome 9 stable?
>
> When you click the publish button, we clear all the saved drafts. :(
> Otherwise we'd continue showing the draft comments alongside the published
> ones. I suppose I should find a way to make that happen when the publish
> actually succeeds instead.
>
>>
>> Thanks,
>>
>> -Ken
>>
>> > -Ken
>> >
>> >> -Expand diff context: You can expand the lines above/below a diff to
>> >> see
>> >> more context.**
>> >> Hope this works well for you all. Obviously, file bugs if something
>> >> isn't
>> >> working.
>> >> Ojan
>> >> * Taking the next step and storing them online for non-security bugs
>> >> (e.g.
>> >> in S3 or appengine) would be a simple and welcome addition if someone
>> >> feels
>> >> moved.
>> >> ** This currently only works if the patch includes the svn revision it
>> >> was
>> >> created at (i.e. it was created with SVN) or applies cleanly to trunk.
>> >> Including the SVN revision in git diffs shouldn't be too hard. Again,
>> >> if you
>> >> feel moved to fix this, ping me.
>> >> _______________________________________________
>> >> webkit-dev mailing list
>> >> webkit-dev at lists.webkit.org
>> >> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> >>
>> >>
>> >
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>