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
