Hi Hans N,
Sure. That would be great.
Thanks
-Nagu
> -----Original Message-----
> From: Hans Nordebäck [mailto:[email protected]]
> Sent: 29 May 2015 14:51
> To: Nagendra Kumar; Anders Widell; Praveen Malviya; Mathivanan Naickan
> Palanivelu
> Cc: [email protected]
> Subject: Re: [PATCH 0 of 1] Review Request for amfd: Add support for google
> unit test framework V2 [#1142]
>
> 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:[email protected]]
> >> Sent: 29 May 2015 13:40
> >> To: Anders Widell; Nagendra Kumar; Praveen Malviya; Mathivanan
> >> Naickan Palanivelu
> >> Cc: [email protected]
> >> 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 <[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
