[PATCH] D21256: Improved Visual Studio 2015 visualization of SmallVectorImpl

[Michael Spertus via cfe-commits]

Hi David,
While I understand the initial reasoning. I have found that this is like a
hundred times better for working on Clang in practice and can't imagine
working without it. The point is that many Clang data structures contain
SmallVectors and having to do zero expansion clicks instead of multiple
each time you take a step through the code is really helpful. If you want
me to back it out and rereview we can, but I'd encourage you to try it out
first.

To ask more about the aside, I'm sorry if I violated community norms. Let
me tell you my reasoning, and you can clarify how I should handle in the
future: Aaron approved me to do post-commit reviews on natvis changes,
which I have done frequently. For this change, I wasn't putting it into
phabricator because I thought pre-commit approval is required but more as a
heads up. Should I change that to be if I don't feel comfortable submitting
without phabricator, then do the full review process?

Thanks,

Mike

On Mon, Jun 13, 2016 at 10:16 AM, David Blaikie <dblaikie at gmail.com> wrote:

> As for the original change proposed: My guiding principle would be "do
> whatever std::vector does". (& that's what I did when implementing GDB
> pretty printers for SmallVector/SmallString/ArrayRef, etc... )
>
> An aside: We generally don't do time limited reviews like this. Either
> something needs review because you're not sure about it, or it doesn't. It
> sounds like the feedback you were looking for probably would've been fine a
> post-commit review feedback just as easily & perhaps might've been a better
> option. (while in this case it was fine - it's sort of a community
> habit/standards thing - we don't want to create the idea that lack of
> feedback is consent/approval in the review process)
>
> On Sun, Jun 12, 2016 at 7:01 PM, Mike Spertus via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> mspertus closed this revision.
>> mspertus added a comment.
>>
>> revision 272525
>>
>>
>> http://reviews.llvm.org/D21256
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160613/8ac78c6c/attachment-0001.html>