Yeah sure, my biggest qualm was the new interface not being implemented. I will reverse the calls and upload the new diff.
On Sat, Feb 15, 2014 at 11:45 PM, Kishore Gopalakrishna <[email protected] > wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18167/#review34590 > ----------------------------------------------------------- > > > Thanks Sandeep for finding the issue and taking a stab at this. I think it > might be better to reverse the calls and default implementations. Only add > onInstaceConfigChange method and call the existing onConfigChange method. > The reason is I think we might have to support additional ConfigChange > listeners such as ClusterConfigChangeListener, ResourceConfigChangeLister > that will simply invoke onConfigChange. > What do you think ? > > - Kishore Gopalakrishna > > > On Feb. 16, 2014, 7:28 a.m., Sandeep Nayak wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/18167/ > > ----------------------------------------------------------- > > > > (Updated Feb. 16, 2014, 7:28 a.m.) > > > > > > Review request for helix. > > > > > > Bugs: HELIX-382 > > > > > > Repository: helix-git > > > > > > Description > > ------- > > > > commit 4c792fd2908cbf8cbfe051dbe31f25bf0a1094df > > Author: Sandeep Nayak <[email protected]> > > Date: Sat Feb 15 23:25:15 2014 -0800 > > > > [HELIX-382] GenericHelixController now implements > InstanceConfigChangeListener, ConfigChangeListener delegates call to > InstanceConfigChangeListener > > > > :100644 100644 e9924a2... 9481802... M > > helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java > > > > > > Diffs > > ----- > > > > > helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java > e9924a2 > > > > Diff: https://reviews.apache.org/r/18167/diff/ > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Sandeep Nayak > > > > > >
