Ack with two comments:

1) I think we should use nanosecond resolution for time measurements. As 
per recommendation from Zoran, this could be implemented as a separate 
ticket, though.
2) Don't add common helper functions in osaf_time.h for reading the 
system time with a low resolution (seconds). Instead, move these helper 
functions to ImmModel and remove them after the ticket mentioned above 
has been implemented.

regards,
Anders Widell

On 03/16/2016 08:36 AM, [email protected] wrote:
> Summary:imm: changing from system time to monotonic time [#1617]
> Review request for Trac Ticket(s): 1617
> Peer Reviewer(s): Zoran, Anders, Hung
> Affected branch(es): latest(5.0)
> Development branch: latest
>
> --------------------------------
> Impacted area       Impact y/n
> --------------------------------
>   Docs                    n
>   Build system            n
>   RPM/packaging           n
>   Configuration files     n
>   Startup scripts         n
>   SAF services            n
>   OpenSAF services        y
>   Core libraries          n
>   Samples                 n
>   Tests                   n
>   Other                   n
>
>
> Comments (indicate scope for each "y" above):
> ---------------------------------------------
>
> changeset 6e89be2a550655417988c77be834e6dfe13349a6
> Author:       Neelakanta Reddy
> Date: Wed, 16 Mar 2016 12:59:21 +0530
>
>       imm: changing from system time to monotonic time [#1617]
>
>
> Complete diffstat:
> ------------------
>   osaf/libs/core/common/include/osaf_time.h    |  20 ++++++++++++++++++++
>   osaf/services/saf/immsv/immnd/ImmModel.cc    |  29 
> ++++++++++++++++-------------
>   osaf/services/saf/immsv/immnd/ImmSearchOp.hh |   3 ++-
>   osaf/services/saf/immsv/immnd/immnd_evt.c    |   7 ++++---
>   osaf/services/saf/immsv/immnd/immnd_proc.c   |   6 ++++--
>   5 files changed, 46 insertions(+), 19 deletions(-)
>
>
> Testing Commands:
> -----------------
> set, the OI timeout to a higher value and try to chnage the
> system time.
>
> Testing, Expected Results:
> --------------------------
> with this patch, the timeout should take number of seconds
> that elapsed, rather than the sytem time seconds.
>
> Conditions of Submission:
> -------------------------
> Ack from Reviewers
>
> 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.
>


------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785231&iu=/4140
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to