Hi Lennart, By mistakenly I included the name of #2214 instead of #2209 in the review request for #2209.
The review request/comments on the patch are referring to #2209 (not #2214). Here we are talking about #2209 only(but the published patch has defect number #2214 ). If you are still confused, please let me know I will re-publish #2209 with correct defect number. Thanks, Neel. On 2016/12/05 05:13 PM, Lennart Lund wrote: > > Hi, > > I am a little bit confused regarding the review of ticket #2209. I was > asked to prioritize reviewing of #2209 and I can see that the ticket > has status “review” but I cannot find any review request for this > ticket. However the patch sent for review for ticket #2214 seems to > contain the code Tai Dinh added as a comment in the #2209 ticket? Also > the #2214 ticket is in state “unassigned”. Can someone please clarify > what the problem is, what patch that solves it and which ticket we are > talking about. Also please fix so that ticket #2214 and #2209 gets the > correct state. > > Thanks > > Lennart > > *From:*Neelakanta Reddy [mailto:reddy.neelaka...@oracle.com] > *Sent:* den 2 december 2016 14:16 > *To:* Tai Chi Dinh <tai.d...@dektech.com.au> > *Cc:* Lennart Lund <lennart.l...@ericsson.com>; Rafael Odzakow > <rafael.odza...@ericsson.com>; opensaf-devel@lists.sourceforge.net > *Subject:* Re: [devel] [PATCH 0 of 1] Review Request for smf:Allow > optimization at node level forAddRemove in mergeStepIntoSingle[#2214] > > Hi Tai, > > forAddRemove we can have the following cases: > 1. only deactivationUnit > 2. only activationUnit > 3. both > > For case 1 and 2 we can not optimize for node/Su/Comp. > For case 3 we can optimize for node/Su/comp. > > I think the published patch needs to be corrected. > i.e if node/SU/Comp is present in both activation and deactivation > then only optimize, otherwise do not optimize. > > Thanks, > Neel. > > > On 2016/12/02 05:44 PM, Tai Chi DINH wrote: > > Hi Neel, > > I think we also need to remove any duplication under SU level and > Component Level also. > Example we have the original campaign that have: > - Rolling on SCs > - ForModify on SU1, SU2 that are hosted on PLs > - ForAddRemoved on SU1, SU2. > > Which this patch, the result campaign will have AU/DU on > SCs/SU1/SU2/SU1/SU2. > Which means we have redundant/unnecessary lock/unlock of SU1/SU2 > (it's enough to just lock/unlock them only once). > > How do you think? > > /Tai > Quoting Neelakanta Reddy <reddy.neelaka...@oracle.com > <mailto:reddy.neelaka...@oracle.com>>: > > Hi All, > > Here the defect no is #2209 not #2214 > > Thanks, > Neel. > > On 2016/12/02 04:22 PM, reddy.neelaka...@oracle.com > <mailto:reddy.neelaka...@oracle.com> wrote: > > Summary: smf:Allow optimization at node level forAddRemove > in mergeStepIntoSingle[#2214] > Review request for Trac Ticket(s): 2214 > Peer Reviewer(s): Rafael, Lennart, tai > Affected branch(es): 5.0.x, 5.1.x, 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): > --------------------------------------------- > > changeset 2becbe07a7f92d70f928e23dcd6b0a6576c8e22a > Author: Neelakanta Reddy > <reddy.neelaka...@oracle.com > <mailto:reddy.neelaka...@oracle.com>> > Date: Fri, 02 Dec 2016 16:16:33 +0530 > > smf:Allow optimization at node level forAddRemove in > mergeStepIntoSingle[#2214] > > > Complete diffstat: > ------------------ > osaf/services/saf/smfsv/smfd/SmfUpgradeProcedure.cc | 40 > +++++++++++++++++++++++++++++++++++++++- > 1 files changed, 39 insertions(+), 1 deletions(-) > > > Testing Commands: > ----------------- > campaign must contain rolling and singlestep upgrade with > AU/SU node level > > Testing, Expected Results: > -------------------------- > Campaign should not fail > > 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. > > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's > most > engaging tech sites, SlashDot.org! > http://sdm.link/slashdot <http://sdm.link/slashdot> > _______________________________________________ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge > > <mailto:Opensaf-devel@lists.sourceforge>.nethttps://lists.sourceforge.net/lists/listinfo/opensaf-devel > > ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
