Code review comment for lp:~dandrader/geis/lp984069

Revision history for this message
Chase Douglas (chasedouglas) wrote :

On 04/24/2012 12:31 PM, Daniel d'Andrada wrote:
>> On 04/24/2012 11:59 AM, Daniel d'Andrada wrote:
>>> ADD_FAILURE() does print the file name and line number but it does *not*
>> return. Therefore a wrapping macro is still needed. So I removed the printf()
>> command and renamed the macro.
>>
>> Ahh, I remembered wrong. You want FAIL() << [message]; It's just like
>> ADD_FAILURE() except that it causes the function to return. It's
>> analogous to ASSERT_*() vs EXPECT_*().
>>
>> http://code.google.com/p/googletest/wiki/AdvancedGuide#Explicit_Success_and_Failure
>
> FAIL(), like ASSERT_*(), can only be used from within functions that return void. That's why I have to stick with ADD_FAILURE().

Ahh, yes, that would be an issue.

This is a bit bike-shedding, but I would prefer to unroll the macro
since it's two simple lines. I think it would make the code easier to
read. However, I'm ok with it as is.

>>
>>> I've removed the --std=c++0x flag from gtest
>>
>> gtest_geis2_grail_backend_CPPFLAGS still has --std=c++0x.
>
> I needs --std=c++0x because it uses c++0x stuff (like nullptr and range-based for loops)

Ok, I didn't realize that.

« Back to merge proposal