@Erik, Thanks!

@hotspot (looking at Robin), can I get another review? 

-- Igor

> On May 24, 2019, at 8:28 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> Thanks, looks good!
> 
> /Erik
> 
> On 2019-05-23 19:07, Igor Ignatyev wrote:
>> Hi Erik,
>> 
>> thanks for your review. I've updated CompileGtest.gmk as you advised[1], 
>> which gives [2].
>> 
>> -- Igor
>> 
>> [1]
>>> diff -r 3443e083223d make/hotspot/lib/CompileGtest.gmk
>>> --- a/make/hotspot/lib/CompileGtest.gmk Thu May 23 19:03:32 2019 -0700
>>> +++ b/make/hotspot/lib/CompileGtest.gmk Thu May 23 19:04:09 2019 -0700
>>> @@ -64,8 +64,9 @@
>>>      EXCLUDES := $(JVM_EXCLUDES), \
>>>      EXCLUDE_FILES := gtestLauncher.cpp, \
>>>      EXCLUDE_PATTERNS := $(JVM_EXCLUDE_PATTERNS), \
>>> -    EXTRA_FILES := $(GTEST_FRAMEWORK_SRC)/googletest/src/gtest-all.cc \
>>> -                   $(GTEST_FRAMEWORK_SRC)/googlemock/src/gmock-all.cc, \
>>> +    EXTRA_FILES := \
>>> +        $(GTEST_FRAMEWORK_SRC)/googletest/src/gtest-all.cc \
>>> +        $(GTEST_FRAMEWORK_SRC)/googlemock/src/gmock-all.cc, \
>>>      EXTRA_OBJECT_FILES := $(filter-out %/operator_new$(OBJ_SUFFIX), \
>>>          $(BUILD_LIBJVM_ALL_OBJS)), \
>>>      CFLAGS := $(JVM_CFLAGS) \
>> [2] http://cr.openjdk.java.net/~iignatyev/8222414/webrev.1-2/
>> 
>>> On May 23, 2019, at 6:57 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
>>> 
>>> Hello Igor,
>>> 
>>> Build changes look ok, with a (very) minor style suggestion [1] (point 18). 
>>> We try to avoid padding continuation line breaks, so I would appreciate if 
>>> CompileGtest.gmk:68 could be indented just 4 extra spaces compared to line 
>>> 67. If you want the file names to line up, it's acceptable to break after 
>>> the := on 67.
>>> 
>>> [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
>>> 
>>> /Erik
>>> 
>>> On 2019-05-22 22:20, Igor Ignatyev wrote:
>>>> http://cr.openjdk.java.net/~iignatyev//8222414/webrev.01
>>>>> 21638 lines changed: 21628 ins; 0 del; 10 mod;
>>>> Hi all,
>>>> 
>>>> could you please review this patch which makes mocking framework from 
>>>> google test available for hotspot tests?
>>>> 
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8222414
>>>> testing: tier1
>>>> webrevs:
>>>>  - http://cr.openjdk.java.net/~iignatyev//8222414/webrev.00
>>>>> 21605 lines changed: 21605 ins; 0 del; 0 mod;
>>>> * moves gtest from test/fmw/gtest to test/fmw/gtest/googletest;
>>>> * adds only required (w/o build-aux, docs, make, msvc, scripts, tests 
>>>> directories and make related files) source of gmock to 
>>>> test/fmw/gtest/googlemock;
>>>> 
>>>> - http://cr.openjdk.java.net/~iignatyev//8222414/webrev.0-1
>>>>> 33 lines changed: 23 ins; 0 del; 10 mod;
>>>> * makes necessary changes in makefiles
>>>> * calls gmock framework init function
>>>> * works around the conflict b/w :testing::internal::Log function defined 
>>>> by gmock and Log macro defined by hotspot
>>>> 
>>>> - http://cr.openjdk.java.net/~iignatyev//8222414/webrev.01
>>>>> 21638 lines changed: 21628 ins; 0 del; 10 mod;
>>>> all the changes together
>>>> 
>>>> 
>>>> thanks,
>>>> -- Igor

Reply via email to