Folks,
I request to please have a look AT IGNITE 1006. It has been ready for a while. Please let me know if anything is needed. |
Hi, see my comments at jira.
-- Artem -- On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> wrote: > Folks, > > I request to please have a look AT IGNITE 1006. It has been ready for a > while. > > Please let me know if anything is needed. > |
As usual, thanks Artem!
Let me look and respond. On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> wrote: > Hi, see my comments at jira. > > -- Artem -- > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> wrote: > > > Folks, > > > > I request to please have a look AT IGNITE 1006. It has been ready for a > > while. > > > > Please let me know if anything is needed. > > > -- Regards, Atri *l'apprenant* |
In reply to this post by Artem Shutak
Added tests.
Please see and let me know your feedback and comments. On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> wrote: > Hi, see my comments at jira. > > -- Artem -- > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> wrote: > > > Folks, > > > > I request to please have a look AT IGNITE 1006. It has been ready for a > > while. > > > > Please let me know if anything is needed. > > > -- Regards, Atri *l'apprenant* |
See my comments for test at jira.
In future, can you please fix all comments before giving new patch for review? -- Artem -- On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> wrote: > Added tests. > > Please see and let me know your feedback and comments. > > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> > wrote: > > > Hi, see my comments at jira. > > > > -- Artem -- > > > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> > wrote: > > > > > Folks, > > > > > > I request to please have a look AT IGNITE 1006. It has been ready for a > > > while. > > > > > > Please let me know if anything is needed. > > > > > > > > > -- > Regards, > > Atri > *l'apprenant* > |
Thanks for your comments.
I am not sure if I understood your comments correctly. For your concern about not reusing ClusterGroupAdapter#nodes, I mentioned that I cannot see a clear mechanism of reusing existing nodes() (without major refactoring of ClusterGroupAdapter#nodes which I try to avoid since ClusterGroupAdapter#nodes is a heavily used method). If there is a method I missed, please let me know. For the tests, test I added gets all the hostnames for test cluster using new method, gets localnode's hostnames and tests if all of localnode's hostnames are present in hostnames result returned by hostNames() method. I did not understand your concern around it (is it that test cluster might have single node)? Please elaborate a bit. Thanks! On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email]> wrote: > See my comments for test at jira. > > In future, can you please fix all comments before giving new patch for > review? > > -- Artem -- > > On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> wrote: > > > Added tests. > > > > Please see and let me know your feedback and comments. > > > > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> > > wrote: > > > > > Hi, see my comments at jira. > > > > > > -- Artem -- > > > > > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> > > wrote: > > > > > > > Folks, > > > > > > > > I request to please have a look AT IGNITE 1006. It has been ready > for a > > > > while. > > > > > > > > Please let me know if anything is needed. > > > > > > > > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > -- Regards, Atri *l'apprenant* |
Hi Artem,
Please let me know your comments on this so I can fix it ASAP. Thanks and Regards, Atri On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> wrote: > Thanks for your comments. > > I am not sure if I understood your comments correctly. > > For your concern about not reusing ClusterGroupAdapter#nodes, I mentioned > that I cannot see a clear mechanism of reusing existing nodes() (without > major refactoring of ClusterGroupAdapter#nodes which I try to avoid since > ClusterGroupAdapter#nodes is a heavily used method). If there is a method I > missed, please let me know. > > For the tests, test I added gets all the hostnames for test cluster using > new method, gets localnode's hostnames and tests if all of localnode's > hostnames are present in hostnames result returned by hostNames() method. I > did not understand your concern around it (is it that test cluster might > have single node)? Please elaborate a bit. > > Thanks! > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email]> > wrote: > >> See my comments for test at jira. >> >> In future, can you please fix all comments before giving new patch for >> review? >> >> -- Artem -- >> >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> >> wrote: >> >> > Added tests. >> > >> > Please see and let me know your feedback and comments. >> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> >> > wrote: >> > >> > > Hi, see my comments at jira. >> > > >> > > -- Artem -- >> > > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> >> > wrote: >> > > >> > > > Folks, >> > > > >> > > > I request to please have a look AT IGNITE 1006. It has been ready >> for a >> > > > while. >> > > > >> > > > Please let me know if anything is needed. >> > > > >> > > >> > >> > >> > >> > -- >> > Regards, >> > >> > Atri >> > *l'apprenant* >> > >> > > > > -- > Regards, > > Atri > *l'apprenant* > -- Regards, Atri *l'apprenant* |
Hi Atri, see my comments at Jira.
-- Artem -- On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> wrote: > Hi Artem, > > > Please let me know your comments on this so I can fix it ASAP. > > Thanks and Regards, > > Atri > > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> wrote: > > > Thanks for your comments. > > > > I am not sure if I understood your comments correctly. > > > > For your concern about not reusing ClusterGroupAdapter#nodes, I mentioned > > that I cannot see a clear mechanism of reusing existing nodes() (without > > major refactoring of ClusterGroupAdapter#nodes which I try to avoid since > > ClusterGroupAdapter#nodes is a heavily used method). If there is a > method I > > missed, please let me know. > > > > For the tests, test I added gets all the hostnames for test cluster using > > new method, gets localnode's hostnames and tests if all of localnode's > > hostnames are present in hostnames result returned by hostNames() > method. I > > did not understand your concern around it (is it that test cluster might > > have single node)? Please elaborate a bit. > > > > Thanks! > > > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email]> > > wrote: > > > >> See my comments for test at jira. > >> > >> In future, can you please fix all comments before giving new patch for > >> review? > >> > >> -- Artem -- > >> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> > >> wrote: > >> > >> > Added tests. > >> > > >> > Please see and let me know your feedback and comments. > >> > > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak <[hidden email]> > >> > wrote: > >> > > >> > > Hi, see my comments at jira. > >> > > > >> > > -- Artem -- > >> > > > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email]> > >> > wrote: > >> > > > >> > > > Folks, > >> > > > > >> > > > I request to please have a look AT IGNITE 1006. It has been ready > >> for a > >> > > > while. > >> > > > > >> > > > Please let me know if anything is needed. > >> > > > > >> > > > >> > > >> > > >> > > >> > -- > >> > Regards, > >> > > >> > Atri > >> > *l'apprenant* > >> > > >> > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > > > > -- > Regards, > > Atri > *l'apprenant* > |
Thanks.
Let me work and revert. On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> wrote: > Hi Atri, see my comments at Jira. > > -- Artem -- > > On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> wrote: > > > Hi Artem, > > > > > > Please let me know your comments on this so I can fix it ASAP. > > > > Thanks and Regards, > > > > Atri > > > > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> > wrote: > > > > > Thanks for your comments. > > > > > > I am not sure if I understood your comments correctly. > > > > > > For your concern about not reusing ClusterGroupAdapter#nodes, I > mentioned > > > that I cannot see a clear mechanism of reusing existing nodes() > (without > > > major refactoring of ClusterGroupAdapter#nodes which I try to avoid > since > > > ClusterGroupAdapter#nodes is a heavily used method). If there is a > > method I > > > missed, please let me know. > > > > > > For the tests, test I added gets all the hostnames for test cluster > using > > > new method, gets localnode's hostnames and tests if all of localnode's > > > hostnames are present in hostnames result returned by hostNames() > > method. I > > > did not understand your concern around it (is it that test cluster > might > > > have single node)? Please elaborate a bit. > > > > > > Thanks! > > > > > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email]> > > > wrote: > > > > > >> See my comments for test at jira. > > >> > > >> In future, can you please fix all comments before giving new patch for > > >> review? > > >> > > >> -- Artem -- > > >> > > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> > > >> wrote: > > >> > > >> > Added tests. > > >> > > > >> > Please see and let me know your feedback and comments. > > >> > > > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > [hidden email]> > > >> > wrote: > > >> > > > >> > > Hi, see my comments at jira. > > >> > > > > >> > > -- Artem -- > > >> > > > > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma <[hidden email] > > > > >> > wrote: > > >> > > > > >> > > > Folks, > > >> > > > > > >> > > > I request to please have a look AT IGNITE 1006. It has been > ready > > >> for a > > >> > > > while. > > >> > > > > > >> > > > Please let me know if anything is needed. > > >> > > > > > >> > > > > >> > > > >> > > > >> > > > >> > -- > > >> > Regards, > > >> > > > >> > Atri > > >> > *l'apprenant* > > >> > > > >> > > > > > > > > > > > > -- > > > Regards, > > > > > > Atri > > > *l'apprenant* > > > > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > -- Regards, Atri *l'apprenant* |
Hi,
I have uploaded next version of patch on ticket. Please see and let me know your comments and feedback. Regards, Atri On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> wrote: > Thanks. > > Let me work and revert. > > On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> > wrote: > >> Hi Atri, see my comments at Jira. >> >> -- Artem -- >> >> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> wrote: >> >> > Hi Artem, >> > >> > >> > Please let me know your comments on this so I can fix it ASAP. >> > >> > Thanks and Regards, >> > >> > Atri >> > >> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> >> wrote: >> > >> > > Thanks for your comments. >> > > >> > > I am not sure if I understood your comments correctly. >> > > >> > > For your concern about not reusing ClusterGroupAdapter#nodes, I >> mentioned >> > > that I cannot see a clear mechanism of reusing existing nodes() >> (without >> > > major refactoring of ClusterGroupAdapter#nodes which I try to avoid >> since >> > > ClusterGroupAdapter#nodes is a heavily used method). If there is a >> > method I >> > > missed, please let me know. >> > > >> > > For the tests, test I added gets all the hostnames for test cluster >> using >> > > new method, gets localnode's hostnames and tests if all of localnode's >> > > hostnames are present in hostnames result returned by hostNames() >> > method. I >> > > did not understand your concern around it (is it that test cluster >> might >> > > have single node)? Please elaborate a bit. >> > > >> > > Thanks! >> > > >> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email]> >> > > wrote: >> > > >> > >> See my comments for test at jira. >> > >> >> > >> In future, can you please fix all comments before giving new patch >> for >> > >> review? >> > >> >> > >> -- Artem -- >> > >> >> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> >> > >> wrote: >> > >> >> > >> > Added tests. >> > >> > >> > >> > Please see and let me know your feedback and comments. >> > >> > >> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < >> [hidden email]> >> > >> > wrote: >> > >> > >> > >> > > Hi, see my comments at jira. >> > >> > > >> > >> > > -- Artem -- >> > >> > > >> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < >> [hidden email]> >> > >> > wrote: >> > >> > > >> > >> > > > Folks, >> > >> > > > >> > >> > > > I request to please have a look AT IGNITE 1006. It has been >> ready >> > >> for a >> > >> > > > while. >> > >> > > > >> > >> > > > Please let me know if anything is needed. >> > >> > > > >> > >> > > >> > >> > >> > >> > >> > >> > >> > >> > -- >> > >> > Regards, >> > >> > >> > >> > Atri >> > >> > *l'apprenant* >> > >> > >> > >> >> > > >> > > >> > > >> > > -- >> > > Regards, >> > > >> > > Atri >> > > *l'apprenant* >> > > >> > >> > >> > >> > -- >> > Regards, >> > >> > Atri >> > *l'apprenant* >> > >> > > > > -- > Regards, > > Atri > *l'apprenant* > -- Regards, Atri *l'apprenant* |
Hi,
Sorry to bug about this. Is there any chance we can get this in sprint 7 please? Please see and let me know your comments. On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > Hi, > > I have uploaded next version of patch on ticket. > > Please see and let me know your comments and feedback. > > Regards, > > Atri > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> wrote: > >> Thanks. >> >> Let me work and revert. >> >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> >> wrote: >> >>> Hi Atri, see my comments at Jira. >>> >>> -- Artem -- >>> >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> >>> wrote: >>> >>> > Hi Artem, >>> > >>> > >>> > Please let me know your comments on this so I can fix it ASAP. >>> > >>> > Thanks and Regards, >>> > >>> > Atri >>> > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> >>> wrote: >>> > >>> > > Thanks for your comments. >>> > > >>> > > I am not sure if I understood your comments correctly. >>> > > >>> > > For your concern about not reusing ClusterGroupAdapter#nodes, I >>> mentioned >>> > > that I cannot see a clear mechanism of reusing existing nodes() >>> (without >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to avoid >>> since >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there is a >>> > method I >>> > > missed, please let me know. >>> > > >>> > > For the tests, test I added gets all the hostnames for test cluster >>> using >>> > > new method, gets localnode's hostnames and tests if all of >>> localnode's >>> > > hostnames are present in hostnames result returned by hostNames() >>> > method. I >>> > > did not understand your concern around it (is it that test cluster >>> might >>> > > have single node)? Please elaborate a bit. >>> > > >>> > > Thanks! >>> > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak <[hidden email] >>> > >>> > > wrote: >>> > > >>> > >> See my comments for test at jira. >>> > >> >>> > >> In future, can you please fix all comments before giving new patch >>> for >>> > >> review? >>> > >> >>> > >> -- Artem -- >>> > >> >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma <[hidden email]> >>> > >> wrote: >>> > >> >>> > >> > Added tests. >>> > >> > >>> > >> > Please see and let me know your feedback and comments. >>> > >> > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < >>> [hidden email]> >>> > >> > wrote: >>> > >> > >>> > >> > > Hi, see my comments at jira. >>> > >> > > >>> > >> > > -- Artem -- >>> > >> > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < >>> [hidden email]> >>> > >> > wrote: >>> > >> > > >>> > >> > > > Folks, >>> > >> > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has been >>> ready >>> > >> for a >>> > >> > > > while. >>> > >> > > > >>> > >> > > > Please let me know if anything is needed. >>> > >> > > > >>> > >> > > >>> > >> > >>> > >> > >>> > >> > >>> > >> > -- >>> > >> > Regards, >>> > >> > >>> > >> > Atri >>> > >> > *l'apprenant* >>> > >> > >>> > >> >>> > > >>> > > >>> > > >>> > > -- >>> > > Regards, >>> > > >>> > > Atri >>> > > *l'apprenant* >>> > > >>> > >>> > >>> > >>> > -- >>> > Regards, >>> > >>> > Atri >>> > *l'apprenant* >>> > >>> >> >> >> >> -- >> Regards, >> >> Atri >> *l'apprenant* >> > > > > -- > Regards, > > Atri > *l'apprenant* > |
Hi Atri,
Sorry, I've fogotten to answer you. I'm okey with your patch in general. So I've created ignite-1006 branch. I will do a little review changes and merge it in sprint-7. It will be done on the next week. I let you know when it will be done. Thanks, Artem. On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> wrote: > Hi, > > Sorry to bug about this. Is there any chance we can get this in sprint 7 > please? > > Please see and let me know your comments. > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > Hi, > > > > I have uploaded next version of patch on ticket. > > > > Please see and let me know your comments and feedback. > > > > Regards, > > > > Atri > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> > wrote: > > > >> Thanks. > >> > >> Let me work and revert. > >> > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> > >> wrote: > >> > >>> Hi Atri, see my comments at Jira. > >>> > >>> -- Artem -- > >>> > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> > >>> wrote: > >>> > >>> > Hi Artem, > >>> > > >>> > > >>> > Please let me know your comments on this so I can fix it ASAP. > >>> > > >>> > Thanks and Regards, > >>> > > >>> > Atri > >>> > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> > >>> wrote: > >>> > > >>> > > Thanks for your comments. > >>> > > > >>> > > I am not sure if I understood your comments correctly. > >>> > > > >>> > > For your concern about not reusing ClusterGroupAdapter#nodes, I > >>> mentioned > >>> > > that I cannot see a clear mechanism of reusing existing nodes() > >>> (without > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to avoid > >>> since > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there is a > >>> > method I > >>> > > missed, please let me know. > >>> > > > >>> > > For the tests, test I added gets all the hostnames for test cluster > >>> using > >>> > > new method, gets localnode's hostnames and tests if all of > >>> localnode's > >>> > > hostnames are present in hostnames result returned by hostNames() > >>> > method. I > >>> > > did not understand your concern around it (is it that test cluster > >>> might > >>> > > have single node)? Please elaborate a bit. > >>> > > > >>> > > Thanks! > >>> > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > [hidden email] > >>> > > >>> > > wrote: > >>> > > > >>> > >> See my comments for test at jira. > >>> > >> > >>> > >> In future, can you please fix all comments before giving new patch > >>> for > >>> > >> review? > >>> > >> > >>> > >> -- Artem -- > >>> > >> > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > [hidden email]> > >>> > >> wrote: > >>> > >> > >>> > >> > Added tests. > >>> > >> > > >>> > >> > Please see and let me know your feedback and comments. > >>> > >> > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > >>> [hidden email]> > >>> > >> > wrote: > >>> > >> > > >>> > >> > > Hi, see my comments at jira. > >>> > >> > > > >>> > >> > > -- Artem -- > >>> > >> > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > >>> [hidden email]> > >>> > >> > wrote: > >>> > >> > > > >>> > >> > > > Folks, > >>> > >> > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has been > >>> ready > >>> > >> for a > >>> > >> > > > while. > >>> > >> > > > > >>> > >> > > > Please let me know if anything is needed. > >>> > >> > > > > >>> > >> > > > >>> > >> > > >>> > >> > > >>> > >> > > >>> > >> > -- > >>> > >> > Regards, > >>> > >> > > >>> > >> > Atri > >>> > >> > *l'apprenant* > >>> > >> > > >>> > >> > >>> > > > >>> > > > >>> > > > >>> > > -- > >>> > > Regards, > >>> > > > >>> > > Atri > >>> > > *l'apprenant* > >>> > > > >>> > > >>> > > >>> > > >>> > -- > >>> > Regards, > >>> > > >>> > Atri > >>> > *l'apprenant* > >>> > > >>> > >> > >> > >> > >> -- > >> Regards, > >> > >> Atri > >> *l'apprenant* > >> > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > |
Thank you so much!
On 26 Jun 2015 00:47, "Artiom Shutak" <[hidden email]> wrote: > Hi Atri, > > Sorry, I've fogotten to answer you. > > I'm okey with your patch in general. So I've created ignite-1006 branch. I > will do a little review changes and merge it in sprint-7. It will be done > on the next week. I let you know when it will be done. > > Thanks, > Artem. > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> wrote: > > > Hi, > > > > Sorry to bug about this. Is there any chance we can get this in sprint 7 > > please? > > > > Please see and let me know your comments. > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > Hi, > > > > > > I have uploaded next version of patch on ticket. > > > > > > Please see and let me know your comments and feedback. > > > > > > Regards, > > > > > > Atri > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> > > wrote: > > > > > >> Thanks. > > >> > > >> Let me work and revert. > > >> > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> > > >> wrote: > > >> > > >>> Hi Atri, see my comments at Jira. > > >>> > > >>> -- Artem -- > > >>> > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> > > >>> wrote: > > >>> > > >>> > Hi Artem, > > >>> > > > >>> > > > >>> > Please let me know your comments on this so I can fix it ASAP. > > >>> > > > >>> > Thanks and Regards, > > >>> > > > >>> > Atri > > >>> > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> > > >>> wrote: > > >>> > > > >>> > > Thanks for your comments. > > >>> > > > > >>> > > I am not sure if I understood your comments correctly. > > >>> > > > > >>> > > For your concern about not reusing ClusterGroupAdapter#nodes, I > > >>> mentioned > > >>> > > that I cannot see a clear mechanism of reusing existing nodes() > > >>> (without > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to > avoid > > >>> since > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there is > a > > >>> > method I > > >>> > > missed, please let me know. > > >>> > > > > >>> > > For the tests, test I added gets all the hostnames for test > cluster > > >>> using > > >>> > > new method, gets localnode's hostnames and tests if all of > > >>> localnode's > > >>> > > hostnames are present in hostnames result returned by hostNames() > > >>> > method. I > > >>> > > did not understand your concern around it (is it that test > cluster > > >>> might > > >>> > > have single node)? Please elaborate a bit. > > >>> > > > > >>> > > Thanks! > > >>> > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > [hidden email] > > >>> > > > >>> > > wrote: > > >>> > > > > >>> > >> See my comments for test at jira. > > >>> > >> > > >>> > >> In future, can you please fix all comments before giving new > patch > > >>> for > > >>> > >> review? > > >>> > >> > > >>> > >> -- Artem -- > > >>> > >> > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > [hidden email]> > > >>> > >> wrote: > > >>> > >> > > >>> > >> > Added tests. > > >>> > >> > > > >>> > >> > Please see and let me know your feedback and comments. > > >>> > >> > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > >>> [hidden email]> > > >>> > >> > wrote: > > >>> > >> > > > >>> > >> > > Hi, see my comments at jira. > > >>> > >> > > > > >>> > >> > > -- Artem -- > > >>> > >> > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > >>> [hidden email]> > > >>> > >> > wrote: > > >>> > >> > > > > >>> > >> > > > Folks, > > >>> > >> > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has > been > > >>> ready > > >>> > >> for a > > >>> > >> > > > while. > > >>> > >> > > > > > >>> > >> > > > Please let me know if anything is needed. > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > >>> > >> > > > >>> > >> > > > >>> > >> > -- > > >>> > >> > Regards, > > >>> > >> > > > >>> > >> > Atri > > >>> > >> > *l'apprenant* > > >>> > >> > > > >>> > >> > > >>> > > > > >>> > > > > >>> > > > > >>> > > -- > > >>> > > Regards, > > >>> > > > > >>> > > Atri > > >>> > > *l'apprenant* > > >>> > > > > >>> > > > >>> > > > >>> > > > >>> > -- > > >>> > Regards, > > >>> > > > >>> > Atri > > >>> > *l'apprenant* > > >>> > > > >>> > > >> > > >> > > >> > > >> -- > > >> Regards, > > >> > > >> Atri > > >> *l'apprenant* > > >> > > > > > > > > > > > > -- > > > Regards, > > > > > > Atri > > > *l'apprenant* > > > > > > |
In reply to this post by Artem Shutak
Hi Artem,
Sorry about bothering but out of curiosity I tried looking up ignite-1006 branch in ignite github but could not find so. Am I looking in right place please? On Fri, Jun 26, 2015 at 12:47 AM, Artiom Shutak <[hidden email]> wrote: > Hi Atri, > > Sorry, I've fogotten to answer you. > > I'm okey with your patch in general. So I've created ignite-1006 branch. I > will do a little review changes and merge it in sprint-7. It will be done > on the next week. I let you know when it will be done. > > Thanks, > Artem. > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> wrote: > > > Hi, > > > > Sorry to bug about this. Is there any chance we can get this in sprint 7 > > please? > > > > Please see and let me know your comments. > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > Hi, > > > > > > I have uploaded next version of patch on ticket. > > > > > > Please see and let me know your comments and feedback. > > > > > > Regards, > > > > > > Atri > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> > > wrote: > > > > > >> Thanks. > > >> > > >> Let me work and revert. > > >> > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak <[hidden email]> > > >> wrote: > > >> > > >>> Hi Atri, see my comments at Jira. > > >>> > > >>> -- Artem -- > > >>> > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> > > >>> wrote: > > >>> > > >>> > Hi Artem, > > >>> > > > >>> > > > >>> > Please let me know your comments on this so I can fix it ASAP. > > >>> > > > >>> > Thanks and Regards, > > >>> > > > >>> > Atri > > >>> > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma <[hidden email]> > > >>> wrote: > > >>> > > > >>> > > Thanks for your comments. > > >>> > > > > >>> > > I am not sure if I understood your comments correctly. > > >>> > > > > >>> > > For your concern about not reusing ClusterGroupAdapter#nodes, I > > >>> mentioned > > >>> > > that I cannot see a clear mechanism of reusing existing nodes() > > >>> (without > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to > avoid > > >>> since > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there is > a > > >>> > method I > > >>> > > missed, please let me know. > > >>> > > > > >>> > > For the tests, test I added gets all the hostnames for test > cluster > > >>> using > > >>> > > new method, gets localnode's hostnames and tests if all of > > >>> localnode's > > >>> > > hostnames are present in hostnames result returned by hostNames() > > >>> > method. I > > >>> > > did not understand your concern around it (is it that test > cluster > > >>> might > > >>> > > have single node)? Please elaborate a bit. > > >>> > > > > >>> > > Thanks! > > >>> > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > [hidden email] > > >>> > > > >>> > > wrote: > > >>> > > > > >>> > >> See my comments for test at jira. > > >>> > >> > > >>> > >> In future, can you please fix all comments before giving new > patch > > >>> for > > >>> > >> review? > > >>> > >> > > >>> > >> -- Artem -- > > >>> > >> > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > [hidden email]> > > >>> > >> wrote: > > >>> > >> > > >>> > >> > Added tests. > > >>> > >> > > > >>> > >> > Please see and let me know your feedback and comments. > > >>> > >> > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > >>> [hidden email]> > > >>> > >> > wrote: > > >>> > >> > > > >>> > >> > > Hi, see my comments at jira. > > >>> > >> > > > > >>> > >> > > -- Artem -- > > >>> > >> > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > >>> [hidden email]> > > >>> > >> > wrote: > > >>> > >> > > > > >>> > >> > > > Folks, > > >>> > >> > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has > been > > >>> ready > > >>> > >> for a > > >>> > >> > > > while. > > >>> > >> > > > > > >>> > >> > > > Please let me know if anything is needed. > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > >>> > >> > > > >>> > >> > > > >>> > >> > -- > > >>> > >> > Regards, > > >>> > >> > > > >>> > >> > Atri > > >>> > >> > *l'apprenant* > > >>> > >> > > > >>> > >> > > >>> > > > > >>> > > > > >>> > > > > >>> > > -- > > >>> > > Regards, > > >>> > > > > >>> > > Atri > > >>> > > *l'apprenant* > > >>> > > > > >>> > > > >>> > > > >>> > > > >>> > -- > > >>> > Regards, > > >>> > > > >>> > Atri > > >>> > *l'apprenant* > > >>> > > > >>> > > >> > > >> > > >> > > >> -- > > >> Regards, > > >> > > >> Atri > > >> *l'apprenant* > > >> > > > > > > > > > > > > -- > > > Regards, > > > > > > Atri > > > *l'apprenant* > > > > > > -- Regards, Atri *l'apprenant* |
Can you recheck please now (do not forget about 'git fetch') ?
-- Artem -- On Fri, Jun 26, 2015 at 10:14 AM, Atri Sharma <[hidden email]> wrote: > Hi Artem, > > Sorry about bothering but out of curiosity I tried looking up ignite-1006 > branch in ignite github but could not find so. > > Am I looking in right place please? > > On Fri, Jun 26, 2015 at 12:47 AM, Artiom Shutak <[hidden email]> > wrote: > > > Hi Atri, > > > > Sorry, I've fogotten to answer you. > > > > I'm okey with your patch in general. So I've created ignite-1006 branch. > I > > will do a little review changes and merge it in sprint-7. It will be done > > on the next week. I let you know when it will be done. > > > > Thanks, > > Artem. > > > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> > wrote: > > > > > Hi, > > > > > > Sorry to bug about this. Is there any chance we can get this in sprint > 7 > > > please? > > > > > > Please see and let me know your comments. > > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > > > Hi, > > > > > > > > I have uploaded next version of patch on ticket. > > > > > > > > Please see and let me know your comments and feedback. > > > > > > > > Regards, > > > > > > > > Atri > > > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> > > > wrote: > > > > > > > >> Thanks. > > > >> > > > >> Let me work and revert. > > > >> > > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak < > [hidden email]> > > > >> wrote: > > > >> > > > >>> Hi Atri, see my comments at Jira. > > > >>> > > > >>> -- Artem -- > > > >>> > > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma <[hidden email]> > > > >>> wrote: > > > >>> > > > >>> > Hi Artem, > > > >>> > > > > >>> > > > > >>> > Please let me know your comments on this so I can fix it ASAP. > > > >>> > > > > >>> > Thanks and Regards, > > > >>> > > > > >>> > Atri > > > >>> > > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma < > [hidden email]> > > > >>> wrote: > > > >>> > > > > >>> > > Thanks for your comments. > > > >>> > > > > > >>> > > I am not sure if I understood your comments correctly. > > > >>> > > > > > >>> > > For your concern about not reusing ClusterGroupAdapter#nodes, I > > > >>> mentioned > > > >>> > > that I cannot see a clear mechanism of reusing existing nodes() > > > >>> (without > > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to > > avoid > > > >>> since > > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there > is > > a > > > >>> > method I > > > >>> > > missed, please let me know. > > > >>> > > > > > >>> > > For the tests, test I added gets all the hostnames for test > > cluster > > > >>> using > > > >>> > > new method, gets localnode's hostnames and tests if all of > > > >>> localnode's > > > >>> > > hostnames are present in hostnames result returned by > hostNames() > > > >>> > method. I > > > >>> > > did not understand your concern around it (is it that test > > cluster > > > >>> might > > > >>> > > have single node)? Please elaborate a bit. > > > >>> > > > > > >>> > > Thanks! > > > >>> > > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > > [hidden email] > > > >>> > > > > >>> > > wrote: > > > >>> > > > > > >>> > >> See my comments for test at jira. > > > >>> > >> > > > >>> > >> In future, can you please fix all comments before giving new > > patch > > > >>> for > > > >>> > >> review? > > > >>> > >> > > > >>> > >> -- Artem -- > > > >>> > >> > > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > > [hidden email]> > > > >>> > >> wrote: > > > >>> > >> > > > >>> > >> > Added tests. > > > >>> > >> > > > > >>> > >> > Please see and let me know your feedback and comments. > > > >>> > >> > > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > > >>> [hidden email]> > > > >>> > >> > wrote: > > > >>> > >> > > > > >>> > >> > > Hi, see my comments at jira. > > > >>> > >> > > > > > >>> > >> > > -- Artem -- > > > >>> > >> > > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > > >>> [hidden email]> > > > >>> > >> > wrote: > > > >>> > >> > > > > > >>> > >> > > > Folks, > > > >>> > >> > > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has > > been > > > >>> ready > > > >>> > >> for a > > > >>> > >> > > > while. > > > >>> > >> > > > > > > >>> > >> > > > Please let me know if anything is needed. > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > >> > > > > >>> > >> > > > > >>> > >> > > > > >>> > >> > -- > > > >>> > >> > Regards, > > > >>> > >> > > > > >>> > >> > Atri > > > >>> > >> > *l'apprenant* > > > >>> > >> > > > > >>> > >> > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> > > -- > > > >>> > > Regards, > > > >>> > > > > > >>> > > Atri > > > >>> > > *l'apprenant* > > > >>> > > > > > >>> > > > > >>> > > > > >>> > > > > >>> > -- > > > >>> > Regards, > > > >>> > > > > >>> > Atri > > > >>> > *l'apprenant* > > > >>> > > > > >>> > > > >> > > > >> > > > >> > > > >> -- > > > >> Regards, > > > >> > > > >> Atri > > > >> *l'apprenant* > > > >> > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Atri > > > > *l'apprenant* > > > > > > > > > > > > > -- > Regards, > > Atri > *l'apprenant* > |
I see it, thank you!
On Fri, Jun 26, 2015 at 2:01 PM, Artiom Shutak <[hidden email]> wrote: > Can you recheck please now (do not forget about 'git fetch') ? > > -- Artem -- > > On Fri, Jun 26, 2015 at 10:14 AM, Atri Sharma <[hidden email]> wrote: > > > Hi Artem, > > > > Sorry about bothering but out of curiosity I tried looking up ignite-1006 > > branch in ignite github but could not find so. > > > > Am I looking in right place please? > > > > On Fri, Jun 26, 2015 at 12:47 AM, Artiom Shutak <[hidden email]> > > wrote: > > > > > Hi Atri, > > > > > > Sorry, I've fogotten to answer you. > > > > > > I'm okey with your patch in general. So I've created ignite-1006 > branch. > > I > > > will do a little review changes and merge it in sprint-7. It will be > done > > > on the next week. I let you know when it will be done. > > > > > > Thanks, > > > Artem. > > > > > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> > > wrote: > > > > > > > Hi, > > > > > > > > Sorry to bug about this. Is there any chance we can get this in > sprint > > 7 > > > > please? > > > > > > > > Please see and let me know your comments. > > > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > > > > > Hi, > > > > > > > > > > I have uploaded next version of patch on ticket. > > > > > > > > > > Please see and let me know your comments and feedback. > > > > > > > > > > Regards, > > > > > > > > > > Atri > > > > > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma <[hidden email]> > > > > wrote: > > > > > > > > > >> Thanks. > > > > >> > > > > >> Let me work and revert. > > > > >> > > > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak < > > [hidden email]> > > > > >> wrote: > > > > >> > > > > >>> Hi Atri, see my comments at Jira. > > > > >>> > > > > >>> -- Artem -- > > > > >>> > > > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma < > [hidden email]> > > > > >>> wrote: > > > > >>> > > > > >>> > Hi Artem, > > > > >>> > > > > > >>> > > > > > >>> > Please let me know your comments on this so I can fix it ASAP. > > > > >>> > > > > > >>> > Thanks and Regards, > > > > >>> > > > > > >>> > Atri > > > > >>> > > > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma < > > [hidden email]> > > > > >>> wrote: > > > > >>> > > > > > >>> > > Thanks for your comments. > > > > >>> > > > > > > >>> > > I am not sure if I understood your comments correctly. > > > > >>> > > > > > > >>> > > For your concern about not reusing > ClusterGroupAdapter#nodes, I > > > > >>> mentioned > > > > >>> > > that I cannot see a clear mechanism of reusing existing > nodes() > > > > >>> (without > > > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try to > > > avoid > > > > >>> since > > > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If there > > is > > > a > > > > >>> > method I > > > > >>> > > missed, please let me know. > > > > >>> > > > > > > >>> > > For the tests, test I added gets all the hostnames for test > > > cluster > > > > >>> using > > > > >>> > > new method, gets localnode's hostnames and tests if all of > > > > >>> localnode's > > > > >>> > > hostnames are present in hostnames result returned by > > hostNames() > > > > >>> > method. I > > > > >>> > > did not understand your concern around it (is it that test > > > cluster > > > > >>> might > > > > >>> > > have single node)? Please elaborate a bit. > > > > >>> > > > > > > >>> > > Thanks! > > > > >>> > > > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > > > [hidden email] > > > > >>> > > > > > >>> > > wrote: > > > > >>> > > > > > > >>> > >> See my comments for test at jira. > > > > >>> > >> > > > > >>> > >> In future, can you please fix all comments before giving new > > > patch > > > > >>> for > > > > >>> > >> review? > > > > >>> > >> > > > > >>> > >> -- Artem -- > > > > >>> > >> > > > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > > > [hidden email]> > > > > >>> > >> wrote: > > > > >>> > >> > > > > >>> > >> > Added tests. > > > > >>> > >> > > > > > >>> > >> > Please see and let me know your feedback and comments. > > > > >>> > >> > > > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > > > >>> [hidden email]> > > > > >>> > >> > wrote: > > > > >>> > >> > > > > > >>> > >> > > Hi, see my comments at jira. > > > > >>> > >> > > > > > > >>> > >> > > -- Artem -- > > > > >>> > >> > > > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > > > >>> [hidden email]> > > > > >>> > >> > wrote: > > > > >>> > >> > > > > > > >>> > >> > > > Folks, > > > > >>> > >> > > > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It has > > > been > > > > >>> ready > > > > >>> > >> for a > > > > >>> > >> > > > while. > > > > >>> > >> > > > > > > > >>> > >> > > > Please let me know if anything is needed. > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > >> > > > > > >>> > >> > -- > > > > >>> > >> > Regards, > > > > >>> > >> > > > > > >>> > >> > Atri > > > > >>> > >> > *l'apprenant* > > > > >>> > >> > > > > > >>> > >> > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >>> > > -- > > > > >>> > > Regards, > > > > >>> > > > > > > >>> > > Atri > > > > >>> > > *l'apprenant* > > > > >>> > > > > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> > -- > > > > >>> > Regards, > > > > >>> > > > > > >>> > Atri > > > > >>> > *l'apprenant* > > > > >>> > > > > > >>> > > > > >> > > > > >> > > > > >> > > > > >> -- > > > > >> Regards, > > > > >> > > > > >> Atri > > > > >> *l'apprenant* > > > > >> > > > > > > > > > > > > > > > > > > > > -- > > > > > Regards, > > > > > > > > > > Atri > > > > > *l'apprenant* > > > > > > > > > > > > > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > -- Regards, Atri *l'apprenant* |
I've done review changes for your patch (see '# ignite-1006: review'
commit) and all changes for ignite-1006 were pushed at sprint-7. I will close the ticket. Thanks for your contribution! -- Artem -- On Fri, Jun 26, 2015 at 11:33 AM, Atri Sharma <[hidden email]> wrote: > I see it, thank you! > > On Fri, Jun 26, 2015 at 2:01 PM, Artiom Shutak <[hidden email]> > wrote: > > > Can you recheck please now (do not forget about 'git fetch') ? > > > > -- Artem -- > > > > On Fri, Jun 26, 2015 at 10:14 AM, Atri Sharma <[hidden email]> > wrote: > > > > > Hi Artem, > > > > > > Sorry about bothering but out of curiosity I tried looking up > ignite-1006 > > > branch in ignite github but could not find so. > > > > > > Am I looking in right place please? > > > > > > On Fri, Jun 26, 2015 at 12:47 AM, Artiom Shutak <[hidden email]> > > > wrote: > > > > > > > Hi Atri, > > > > > > > > Sorry, I've fogotten to answer you. > > > > > > > > I'm okey with your patch in general. So I've created ignite-1006 > > branch. > > > I > > > > will do a little review changes and merge it in sprint-7. It will be > > done > > > > on the next week. I let you know when it will be done. > > > > > > > > Thanks, > > > > Artem. > > > > > > > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email]> > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > Sorry to bug about this. Is there any chance we can get this in > > sprint > > > 7 > > > > > please? > > > > > > > > > > Please see and let me know your comments. > > > > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > I have uploaded next version of patch on ticket. > > > > > > > > > > > > Please see and let me know your comments and feedback. > > > > > > > > > > > > Regards, > > > > > > > > > > > > Atri > > > > > > > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma < > [hidden email]> > > > > > wrote: > > > > > > > > > > > >> Thanks. > > > > > >> > > > > > >> Let me work and revert. > > > > > >> > > > > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak < > > > [hidden email]> > > > > > >> wrote: > > > > > >> > > > > > >>> Hi Atri, see my comments at Jira. > > > > > >>> > > > > > >>> -- Artem -- > > > > > >>> > > > > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma < > > [hidden email]> > > > > > >>> wrote: > > > > > >>> > > > > > >>> > Hi Artem, > > > > > >>> > > > > > > >>> > > > > > > >>> > Please let me know your comments on this so I can fix it > ASAP. > > > > > >>> > > > > > > >>> > Thanks and Regards, > > > > > >>> > > > > > > >>> > Atri > > > > > >>> > > > > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma < > > > [hidden email]> > > > > > >>> wrote: > > > > > >>> > > > > > > >>> > > Thanks for your comments. > > > > > >>> > > > > > > > >>> > > I am not sure if I understood your comments correctly. > > > > > >>> > > > > > > > >>> > > For your concern about not reusing > > ClusterGroupAdapter#nodes, I > > > > > >>> mentioned > > > > > >>> > > that I cannot see a clear mechanism of reusing existing > > nodes() > > > > > >>> (without > > > > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I try > to > > > > avoid > > > > > >>> since > > > > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If > there > > > is > > > > a > > > > > >>> > method I > > > > > >>> > > missed, please let me know. > > > > > >>> > > > > > > > >>> > > For the tests, test I added gets all the hostnames for test > > > > cluster > > > > > >>> using > > > > > >>> > > new method, gets localnode's hostnames and tests if all of > > > > > >>> localnode's > > > > > >>> > > hostnames are present in hostnames result returned by > > > hostNames() > > > > > >>> > method. I > > > > > >>> > > did not understand your concern around it (is it that test > > > > cluster > > > > > >>> might > > > > > >>> > > have single node)? Please elaborate a bit. > > > > > >>> > > > > > > > >>> > > Thanks! > > > > > >>> > > > > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > > > > [hidden email] > > > > > >>> > > > > > > >>> > > wrote: > > > > > >>> > > > > > > > >>> > >> See my comments for test at jira. > > > > > >>> > >> > > > > > >>> > >> In future, can you please fix all comments before giving > new > > > > patch > > > > > >>> for > > > > > >>> > >> review? > > > > > >>> > >> > > > > > >>> > >> -- Artem -- > > > > > >>> > >> > > > > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > > > > [hidden email]> > > > > > >>> > >> wrote: > > > > > >>> > >> > > > > > >>> > >> > Added tests. > > > > > >>> > >> > > > > > > >>> > >> > Please see and let me know your feedback and comments. > > > > > >>> > >> > > > > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > > > > >>> [hidden email]> > > > > > >>> > >> > wrote: > > > > > >>> > >> > > > > > > >>> > >> > > Hi, see my comments at jira. > > > > > >>> > >> > > > > > > > >>> > >> > > -- Artem -- > > > > > >>> > >> > > > > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > > > > >>> [hidden email]> > > > > > >>> > >> > wrote: > > > > > >>> > >> > > > > > > > >>> > >> > > > Folks, > > > > > >>> > >> > > > > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It > has > > > > been > > > > > >>> ready > > > > > >>> > >> for a > > > > > >>> > >> > > > while. > > > > > >>> > >> > > > > > > > > >>> > >> > > > Please let me know if anything is needed. > > > > > >>> > >> > > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > >> > > > > > > >>> > >> > > > > > > >>> > >> > -- > > > > > >>> > >> > Regards, > > > > > >>> > >> > > > > > > >>> > >> > Atri > > > > > >>> > >> > *l'apprenant* > > > > > >>> > >> > > > > > > >>> > >> > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > -- > > > > > >>> > > Regards, > > > > > >>> > > > > > > > >>> > > Atri > > > > > >>> > > *l'apprenant* > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >>> > -- > > > > > >>> > Regards, > > > > > >>> > > > > > > >>> > Atri > > > > > >>> > *l'apprenant* > > > > > >>> > > > > > > >>> > > > > > >> > > > > > >> > > > > > >> > > > > > >> -- > > > > > >> Regards, > > > > > >> > > > > > >> Atri > > > > > >> *l'apprenant* > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Regards, > > > > > > > > > > > > Atri > > > > > > *l'apprenant* > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > Regards, > > > > > > Atri > > > *l'apprenant* > > > > > > > > > -- > Regards, > > Atri > *l'apprenant* > |
Thank you so much again!
On Fri, Jun 26, 2015 at 8:42 PM, Artiom Shutak <[hidden email]> wrote: > I've done review changes for your patch (see '# ignite-1006: review' > commit) and all changes for ignite-1006 were pushed at sprint-7. > > I will close the ticket. > > Thanks for your contribution! > > -- Artem -- > > On Fri, Jun 26, 2015 at 11:33 AM, Atri Sharma <[hidden email]> wrote: > > > I see it, thank you! > > > > On Fri, Jun 26, 2015 at 2:01 PM, Artiom Shutak <[hidden email]> > > wrote: > > > > > Can you recheck please now (do not forget about 'git fetch') ? > > > > > > -- Artem -- > > > > > > On Fri, Jun 26, 2015 at 10:14 AM, Atri Sharma <[hidden email]> > > wrote: > > > > > > > Hi Artem, > > > > > > > > Sorry about bothering but out of curiosity I tried looking up > > ignite-1006 > > > > branch in ignite github but could not find so. > > > > > > > > Am I looking in right place please? > > > > > > > > On Fri, Jun 26, 2015 at 12:47 AM, Artiom Shutak < > [hidden email]> > > > > wrote: > > > > > > > > > Hi Atri, > > > > > > > > > > Sorry, I've fogotten to answer you. > > > > > > > > > > I'm okey with your patch in general. So I've created ignite-1006 > > > branch. > > > > I > > > > > will do a little review changes and merge it in sprint-7. It will > be > > > done > > > > > on the next week. I let you know when it will be done. > > > > > > > > > > Thanks, > > > > > Artem. > > > > > > > > > > On Thu, Jun 25, 2015 at 10:03 PM, Atri Sharma <[hidden email] > > > > > > wrote: > > > > > > > > > > > Hi, > > > > > > > > > > > > Sorry to bug about this. Is there any chance we can get this in > > > sprint > > > > 7 > > > > > > please? > > > > > > > > > > > > Please see and let me know your comments. > > > > > > On 25 Jun 2015 14:19, "Atri Sharma" <[hidden email]> wrote: > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > I have uploaded next version of patch on ticket. > > > > > > > > > > > > > > Please see and let me know your comments and feedback. > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > Atri > > > > > > > > > > > > > > On Wed, Jun 24, 2015 at 5:48 PM, Atri Sharma < > > [hidden email]> > > > > > > wrote: > > > > > > > > > > > > > >> Thanks. > > > > > > >> > > > > > > >> Let me work and revert. > > > > > > >> > > > > > > >> On Wed, Jun 24, 2015 at 5:44 PM, Artiom Shutak < > > > > [hidden email]> > > > > > > >> wrote: > > > > > > >> > > > > > > >>> Hi Atri, see my comments at Jira. > > > > > > >>> > > > > > > >>> -- Artem -- > > > > > > >>> > > > > > > >>> On Wed, Jun 24, 2015 at 1:15 PM, Atri Sharma < > > > [hidden email]> > > > > > > >>> wrote: > > > > > > >>> > > > > > > >>> > Hi Artem, > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > Please let me know your comments on this so I can fix it > > ASAP. > > > > > > >>> > > > > > > > >>> > Thanks and Regards, > > > > > > >>> > > > > > > > >>> > Atri > > > > > > >>> > > > > > > > >>> > On Tue, Jun 23, 2015 at 5:08 PM, Atri Sharma < > > > > [hidden email]> > > > > > > >>> wrote: > > > > > > >>> > > > > > > > >>> > > Thanks for your comments. > > > > > > >>> > > > > > > > > >>> > > I am not sure if I understood your comments correctly. > > > > > > >>> > > > > > > > > >>> > > For your concern about not reusing > > > ClusterGroupAdapter#nodes, I > > > > > > >>> mentioned > > > > > > >>> > > that I cannot see a clear mechanism of reusing existing > > > nodes() > > > > > > >>> (without > > > > > > >>> > > major refactoring of ClusterGroupAdapter#nodes which I > try > > to > > > > > avoid > > > > > > >>> since > > > > > > >>> > > ClusterGroupAdapter#nodes is a heavily used method). If > > there > > > > is > > > > > a > > > > > > >>> > method I > > > > > > >>> > > missed, please let me know. > > > > > > >>> > > > > > > > > >>> > > For the tests, test I added gets all the hostnames for > test > > > > > cluster > > > > > > >>> using > > > > > > >>> > > new method, gets localnode's hostnames and tests if all > of > > > > > > >>> localnode's > > > > > > >>> > > hostnames are present in hostnames result returned by > > > > hostNames() > > > > > > >>> > method. I > > > > > > >>> > > did not understand your concern around it (is it that > test > > > > > cluster > > > > > > >>> might > > > > > > >>> > > have single node)? Please elaborate a bit. > > > > > > >>> > > > > > > > > >>> > > Thanks! > > > > > > >>> > > > > > > > > >>> > > On Tue, Jun 23, 2015 at 4:54 PM, Artiom Shutak < > > > > > > [hidden email] > > > > > > >>> > > > > > > > >>> > > wrote: > > > > > > >>> > > > > > > > > >>> > >> See my comments for test at jira. > > > > > > >>> > >> > > > > > > >>> > >> In future, can you please fix all comments before giving > > new > > > > > patch > > > > > > >>> for > > > > > > >>> > >> review? > > > > > > >>> > >> > > > > > > >>> > >> -- Artem -- > > > > > > >>> > >> > > > > > > >>> > >> On Tue, Jun 23, 2015 at 12:50 PM, Atri Sharma < > > > > > > [hidden email]> > > > > > > >>> > >> wrote: > > > > > > >>> > >> > > > > > > >>> > >> > Added tests. > > > > > > >>> > >> > > > > > > > >>> > >> > Please see and let me know your feedback and comments. > > > > > > >>> > >> > > > > > > > >>> > >> > On Mon, Jun 22, 2015 at 4:09 PM, Artiom Shutak < > > > > > > >>> [hidden email]> > > > > > > >>> > >> > wrote: > > > > > > >>> > >> > > > > > > > >>> > >> > > Hi, see my comments at jira. > > > > > > >>> > >> > > > > > > > > >>> > >> > > -- Artem -- > > > > > > >>> > >> > > > > > > > > >>> > >> > > On Fri, Jun 19, 2015 at 6:43 PM, Atri Sharma < > > > > > > >>> [hidden email]> > > > > > > >>> > >> > wrote: > > > > > > >>> > >> > > > > > > > > >>> > >> > > > Folks, > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > I request to please have a look AT IGNITE 1006. It > > has > > > > > been > > > > > > >>> ready > > > > > > >>> > >> for a > > > > > > >>> > >> > > > while. > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > Please let me know if anything is needed. > > > > > > >>> > >> > > > > > > > > > >>> > >> > > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > > >>> > >> > -- > > > > > > >>> > >> > Regards, > > > > > > >>> > >> > > > > > > > >>> > >> > Atri > > > > > > >>> > >> > *l'apprenant* > > > > > > >>> > >> > > > > > > > >>> > >> > > > > > > >>> > > > > > > > > >>> > > > > > > > > >>> > > > > > > > > >>> > > -- > > > > > > >>> > > Regards, > > > > > > >>> > > > > > > > > >>> > > Atri > > > > > > >>> > > *l'apprenant* > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > -- > > > > > > >>> > Regards, > > > > > > >>> > > > > > > > >>> > Atri > > > > > > >>> > *l'apprenant* > > > > > > >>> > > > > > > > >>> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> -- > > > > > > >> Regards, > > > > > > >> > > > > > > >> Atri > > > > > > >> *l'apprenant* > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Regards, > > > > > > > > > > > > > > Atri > > > > > > > *l'apprenant* > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Atri > > > > *l'apprenant* > > > > > > > > > > > > > > > -- > > Regards, > > > > Atri > > *l'apprenant* > > > -- Regards, Atri *l'apprenant* |
Free forum by Nabble | Edit this page |