[webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Adam Barth
Based on some feedback, I'm going to try to improve the line-by-line review tool. I've landed the first iteration of the new design, which should be usable and have roughly the same functionality as the old design. I'll be adding new features shortly. The main difference is you now access the

Re: [webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Oliver Hunt
It would be nice if you could select a block of code and have the comment be for that block -- last i looked the line by line review basically loses context for reviews as it pushes the comments to the bottom and only includes a single line of context. --Oliver On Aug 29, 2010, at 11:13 AM,

Re: [webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Adam Barth
So, I have two thoughts on that: 1) We can certainly do that. The trick will be making it discoverable. 2) I'd like the tool to read back in the state from the bug comments and re-populate the comments inline in the diff. That way you'll keep the context and can have threaded conversations in

Re: [webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Maciej Stachowiak
On Aug 29, 2010, at 4:39 PM, Adam Barth wrote: On Sun, Aug 29, 2010 at 4:16 PM, Maciej Stachowiak m...@apple.com wrote: I'm not sure who objects to new features being added to Review Patch, but I don't like this change: There's a tention between folks who like line-by-line comments and

Re: [webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Adam Barth
On Sun, Aug 29, 2010 at 5:07 PM, Maciej Stachowiak m...@apple.com wrote: On Aug 29, 2010, at 4:39 PM, Adam Barth wrote: I'm happy to build this off in a silo and then convince everyone how awesome it is once it actually is more awesome than the current tools. I suggest you start by making it

Re: [webkit-dev] Changes to line-by-line code reviews

2010-08-29 Thread Darin Adler
On Aug 29, 2010, at 4:39 PM, Adam Barth wrote: Darin likes the giant textbox with the whole diff. Just to give context here: Some day the new tool might be good enough that I’ll change my mind. I use the new tool for about 1/10 of the patches I review and I plan to switch full time once the