Re: [webkit-dev] Experimental new code reviews
Here's the prototype I made during the hackathon that simply uses a bunch of Javascript to make it easier to type up comments: http://ascherkus.appspot.com/bugzilla/index.html I'm not a WebKit reviewer, but from what I gathered during the session there's a lot of copying and pasting involved. I focused on trying to fix that particular issue. Double click a line to add a comment. Code context + comment gets appended into the patch comment box. It's purely visual -- nothing gets persisted. Andrew On Mon, Apr 19, 2010 at 4:20 PM, Ojan Vafai o...@chromium.org wrote: I don't know if it's up anywhere. The other group's approach adds more directly upon the current review system. I don't think we need to choose one vs. another (at least not in the short term). Not that you were suggesting that. Ojan On Mon, Apr 19, 2010 at 4:06 PM, Adam Barth aba...@webkit.org wrote: +scherkus On Mon, Apr 19, 2010 at 4:01 PM, Maciej Stachowiak m...@apple.com wrote: I heard another group coded up a different approach to improving reviews - does anyone have a URL for that, so we can compare? Cheers, Maciej On Apr 19, 2010, at 3:35 PM, Ojan Vafai wrote: At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531. You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org. We do not intend to remove the old code review system for people who prefer to stick to that. Known issues: -Currently, only works with patches that are uploaded using webkit-patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental new code reviews
At the risk of inspiring friendly competition, I've turned Andrew's demo into a patch: https://bugs.webkit.org/show_bug.cgi?id=37886 Adam On Tue, Apr 20, 2010 at 12:24 PM, Andrew Scherkus scher...@chromium.org wrote: Here's the prototype I made during the hackathon that simply uses a bunch of Javascript to make it easier to type up comments: http://ascherkus.appspot.com/bugzilla/index.html I'm not a WebKit reviewer, but from what I gathered during the session there's a lot of copying and pasting involved. I focused on trying to fix that particular issue. Double click a line to add a comment. Code context + comment gets appended into the patch comment box. It's purely visual -- nothing gets persisted. Andrew On Mon, Apr 19, 2010 at 4:20 PM, Ojan Vafai o...@chromium.org wrote: I don't know if it's up anywhere. The other group's approach adds more directly upon the current review system. I don't think we need to choose one vs. another (at least not in the short term). Not that you were suggesting that. Ojan On Mon, Apr 19, 2010 at 4:06 PM, Adam Barth aba...@webkit.org wrote: +scherkus On Mon, Apr 19, 2010 at 4:01 PM, Maciej Stachowiak m...@apple.com wrote: I heard another group coded up a different approach to improving reviews - does anyone have a URL for that, so we can compare? Cheers, Maciej On Apr 19, 2010, at 3:35 PM, Ojan Vafai wrote: At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531. You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org. We do not intend to remove the old code review system for people who prefer to stick to that. Known issues: -Currently, only works with patches that are uploaded using webkit-patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Experimental new code reviews
At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531. You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org . *We do not intend to remove the old code review system for people who prefer to stick to that. * Known issues: -Currently, only works with patches that are uploaded using webkit-patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental new code reviews
I heard another group coded up a different approach to improving reviews - does anyone have a URL for that, so we can compare? Cheers, Maciej On Apr 19, 2010, at 3:35 PM, Ojan Vafai wrote: At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531 . You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org. We do not intend to remove the old code review system for people who prefer to stick to that. Known issues: -Currently, only works with patches that are uploaded using webkit- patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental new code reviews
+scherkus On Mon, Apr 19, 2010 at 4:01 PM, Maciej Stachowiak m...@apple.com wrote: I heard another group coded up a different approach to improving reviews - does anyone have a URL for that, so we can compare? Cheers, Maciej On Apr 19, 2010, at 3:35 PM, Ojan Vafai wrote: At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531. You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org. We do not intend to remove the old code review system for people who prefer to stick to that. Known issues: -Currently, only works with patches that are uploaded using webkit-patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Experimental new code reviews
I don't know if it's up anywhere. The other group's approach adds more directly upon the current review system. I don't think we need to choose one vs. another (at least not in the short term). Not that you were suggesting that. Ojan On Mon, Apr 19, 2010 at 4:06 PM, Adam Barth aba...@webkit.org wrote: +scherkus On Mon, Apr 19, 2010 at 4:01 PM, Maciej Stachowiak m...@apple.com wrote: I heard another group coded up a different approach to improving reviews - does anyone have a URL for that, so we can compare? Cheers, Maciej On Apr 19, 2010, at 3:35 PM, Ojan Vafai wrote: At the hackathon last Tuesday, a few of us put together mashup style rietveld integration with bugs.webkit.org. It currently requires a chrome extension. We'll integrate properly with bugzilla based on feedback if this seems to be a value add for the project. http://webkit-rietveld.googlecode.com/svn/trunk/chrome-extension/webkit-cr.crx You can try it out on the *last* attachment on https://bugs.webkit.org/show_bug.cgi?id=37531. You'll see another link next to each attachment labelled Fancy Review. This loads a page much like the current review page, but with wkrietveld.appspot.com in the top frame (wkrietveld is our fork of rietveld). You can then make comments in rietveld. When you click the submit button, the comments are published *both* in Reitveld and to bugs.webkit.org. We do not intend to remove the old code review system for people who prefer to stick to that. Known issues: -Currently, only works with patches that are uploaded using webkit-patch upload --fancy-review. -Due to using a chrome extension rather than a tighter integration, some things are a bit janky (e.g. the initial load). -Each time a patch is uploaded, it currently creates a new rietveld issue. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
