Hi Nagu,

no problem, I'll wait. Do you want me to send out the latest patches 
updated with AndersW comments?
/Thanks HansN

On 05/29/2015 10:33 AM, Nagendra Kumar wrote:
> Hi Hans N,
>               Sorry for late entry. Can you please hold on until next week, I 
> need to get some more familiarity.
>
> Thanks
> -Nagu
>
>> -----Original Message-----
>> From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com]
>> Sent: 29 May 2015 13:40
>> To: Anders Widell; Nagendra Kumar; Praveen Malviya; Mathivanan Naickan
>> Palanivelu
>> Cc: opensaf-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 0 of 1] Review Request for amfd: Add support for google
>> unit test framework V2 [#1142]
>>
>> 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 <hans.nordeb...@ericsson.com>
>>>> 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
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to