yes I agree it is better to download and build gtest yourself. I have 
incorporated your suggested changes,
it works fine. Is it ok to push these patches now?

/Thanks HansN

On 05/26/2015 02:59 PM, Anders Widell wrote:
> Ack (for both patches on this ticket), with some comments:
>
> * Boiler plates (licence & copyright) are missing in the new files
> * The README assumes the developer uses Ubuntu and has sudo rights 
> (root access). Instead, I think it would be better to describe how to 
> download the Google test framework from the web and build it. This 
> would work on any Linux distribution and does not require root access. 
> I.e. something like this:
>
> wget https://googletest.googlecode.com/files/gtest-1.7.0.zip
> unzip gtest-1.7.0.zip
> cd gtest-1.7.0
> ./configure
> make
> export GTEST_DIR=`pwd`
>
> * Based on the instructions above, we also need to add 
> -L$(GTEST_DIR)/lib to LDFLAGS in the Makefile, since libgtest.a is not 
> installed in /usr/lib
> * When I tested this on Ubuntu 15.04 and gtest 1.7.0, I had to do some 
> modifications to make it build successfully. The attached file shows 
> what I had to change to make it build (including the 
> -L$(GTEST_DIR)/lib mentioned above).
>
> / Anders W
>
> On 05/08/2015 10:11 AM, Hans Nordeback wrote:
>> Summary: amfd: Add support for google unit test framework V2
>> Review request for Trac Ticket(s): #1142
>> Peer Reviewer(s): Praveen, Nagu, Mathi, AndersW
>> Pull request to:
>> Affected branch(es): default
>> Development branch: default
>>
>> --------------------------------
>> Impacted area       Impact y/n
>> --------------------------------
>>   Docs                    n
>>   Build system            n
>>   RPM/packaging           n
>>   Configuration files     n
>>   Startup scripts         n
>>   SAF services            y
>>   OpenSAF services        n
>>   Core libraries          n
>>   Samples                 n
>>   Tests                   n
>>   Other                   n
>>
>>
>> Comments (indicate scope for each "y" above):
>> ---------------------------------------------
>>   <<EXPLAIN/COMMENT THE PATCH SERIES HERE>>
>>
>> changeset 6be42cd2de89b3a8b8e282d35e996e947eedb564
>> Author:    Hans Nordeback <[email protected]>
>> Date:    Fri, 08 May 2015 10:08:19 +0200
>>
>>     amfd: Add support for google unit test framework V2 [#1142]
>>
>>     As part of refactoring enable the use of google unit test framework.
>>
>>
>> Added Files:
>> ------------
>>   00-README.unittest
>>   osaf/services/saf/amf/amfd/tests/Makefile.am
>>   osaf/services/saf/amf/amfd/tests/test_amf_db.cc
>>
>>
>> Complete diffstat:
>> ------------------
>>   00-README.unittest                              |  32 ++++++++++++++++
>>   configure.ac                                    |   1 +
>>   osaf/services/saf/amf/amfd/Makefile.am          |   2 +-
>>   osaf/services/saf/amf/amfd/tests/Makefile.am    |  50 
>> +++++++++++++++++++++++++
>>   osaf/services/saf/amf/amfd/tests/test_amf_db.cc |  44 
>> ++++++++++++++++++++++
>>   5 files changed, 128 insertions(+), 1 deletions(-)
>>
>>
>> Testing Commands:
>> -----------------
>>   <<LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES>>
>>
>>
>> Testing, Expected Results:
>> --------------------------
>>   <<PASTE COMMAND OUTPUTS / TEST RESULTS>>
>>
>>
>> Conditions of Submission:
>> -------------------------
>>   <<HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC>>
>>
>>
>> Arch      Built     Started    Linux distro
>> -------------------------------------------
>> mips        n          n
>> mips64      n          n
>> x86         n          n
>> x86_64      y          y
>> powerpc     n          n
>> powerpc64   n          n
>>
>>
>> Reviewer Checklist:
>> -------------------
>> [Submitters: make sure that your review doesn't trigger any checkmarks!]
>>
>>
>> Your checkin has not passed review because (see checked entries):
>>
>> ___ Your RR template is generally incomplete; it has too many blank 
>> entries
>>      that need proper data filled in.
>>
>> ___ You have failed to nominate the proper persons for review and push.
>>
>> ___ Your patches do not have proper short+long header
>>
>> ___ You have grammar/spelling in your header that is unacceptable.
>>
>> ___ You have exceeded a sensible line length in your 
>> headers/comments/text.
>>
>> ___ You have failed to put in a proper Trac Ticket # into your commits.
>>
>> ___ You have incorrectly put/left internal data in your comments/files
>>      (i.e. internal bug tracking tool IDs, product names etc)
>>
>> ___ You have not given any evidence of testing beyond basic build tests.
>>      Demonstrate some level of runtime or other sanity testing.
>>
>> ___ You have ^M present in some of your files. These have to be removed.
>>
>> ___ You have needlessly changed whitespace or added whitespace crimes
>>      like trailing spaces, or spaces before tabs.
>>
>> ___ You have mixed real technical changes with whitespace and other
>>      cosmetic code cleanup changes. These have to be separate commits.
>>
>> ___ You need to refactor your submission into logical chunks; there is
>>      too much content into a single commit.
>>
>> ___ You have extraneous garbage in your review (merge commits etc)
>>
>> ___ You have giant attachments which should never have been sent;
>>      Instead you should place your content in a public tree to be 
>> pulled.
>>
>> ___ You have too many commits attached to an e-mail; resend as threaded
>>      commits, or place in a public tree for a pull.
>>
>> ___ You have resent this content multiple times without a clear 
>> indication
>>      of what has changed between each re-send.
>>
>> ___ You have failed to adequately and individually address all of the
>>      comments and change requests that were proposed in the initial 
>> review.
>>
>> ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc)
>>
>> ___ Your computer have a badly configured date and time; confusing the
>>      the threaded patch review.
>>
>> ___ Your changes affect IPC mechanism, and you don't present any results
>>      for in-service upgradability test.
>>
>> ___ Your changes affect user manual and documentation, your patch series
>>      do not contain the patch that updates the Doxygen manual.
>>
>


------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to