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
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,
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
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
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
On 08/26/2010 11:56 PM, Holger Freyther wrote:
Hi,
the only change is to exclude the coverage option for ANGLE as libtool does
not like these parameters. I have tested the scripts and they work, I will
need to test the whole generate-coverage script tomorrow and will open a bug
report...
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
On Aug 28, 2010, at 10:57 PM, Darin Adler wrote:
We need VectorOwnPtr too. It has similar issues to HashMap with OwnPtr
values, including the ones mentioned by Maciej.
For one example, look at CSSParser::m_floatingMediaQueryExpList.
VectorOwnPtr actually works[1], and I have an almost
On Aug 29, 2010, at 9:14 PM, Maciej Stachowiak wrote:
Yet another possibility is to use a hash to do the de-duping instead of
sorting; I can't tell from context if the sorting is needed for any purpose
other than subsequent de-duping.
Turns out this doesn't work - the CSS Media Queries
9 matches
Mail list logo
