IGNITE-6879
Locked
123
123
 
|
Hello!
I want to work on https://issues.apache.org/jira/browse/IGNITE-6879 issue. Following the rules here https://ignite.apache.org/community/contribute.html#contribute my Jira username is "homich" so assign me to this ticket please. P.S. I found this rules page after I made PR, so sorry for this inconvenience. Regards. |
|
|
Hello Roman,
It's not a big deal, I've added you to the contributors' list in JIRA. Go ahead and assign the ticket to yourself. Folks, anyway, who can review Roman's contribution that brings Spring 2.0 support to Ignite? -- Denis On Tue, Mar 13, 2018 at 2:21 PM, Роман Меерсон <[hidden email]> wrote: > Hello! > > I want to work on https://issues.apache.org/jira/browse/IGNITE-6879 issue. > Following the rules here > https://ignite.apache.org/community/contribute.html#contribute my Jira > username is "homich" so assign me to this ticket please. > > P.S. I found this rules page after I made PR, so sorry for this > inconvenience. > > Regards. > |
|
Igniters,
it is about spring-data integration support. Who has intereset about styding this technology? Alexey Kukushkin would you like to be reviewer of this issue? Sincerely, Dmitriy Pavlov ср, 14 мар. 2018 г. в 1:08, Denis Magda <[hidden email]>: > Hello Roman, > > It's not a big deal, I've added you to the contributors' list in JIRA. Go > ahead and assign the ticket to yourself. > > Folks, anyway, who can review Roman's contribution that brings Spring 2.0 > support to Ignite? > > -- > Denis > > > > On Tue, Mar 13, 2018 at 2:21 PM, Роман Меерсон <[hidden email]> > wrote: > > > Hello! > > > > I want to work on https://issues.apache.org/jira/browse/IGNITE-6879 > issue. > > Following the rules here > > https://ignite.apache.org/community/contribute.html#contribute my Jira > > username is "homich" so assign me to this ticket please. > > > > P.S. I found this rules page after I made PR, so sorry for this > > inconvenience. > > > > Regards. > > > |
|
|
Dmitry, Roman, I can review the fix - send me a code review link when it is
ready. On Wed, Mar 14, 2018 at 5:14 PM, Dmitry Pavlov <[hidden email]> wrote: > Igniters, > > it is about spring-data integration support. > > Who has intereset about styding this technology? > > Alexey Kukushkin would you like to be reviewer of this issue? > > Sincerely, > Dmitriy Pavlov > > ср, 14 мар. 2018 г. в 1:08, Denis Magda <[hidden email]>: > >> Hello Roman, >> >> It's not a big deal, I've added you to the contributors' list in JIRA. Go >> ahead and assign the ticket to yourself. >> >> Folks, anyway, who can review Roman's contribution that brings Spring 2.0 >> support to Ignite? >> >> -- >> Denis >> >> >> >> On Tue, Mar 13, 2018 at 2:21 PM, Роман Меерсон <[hidden email]> >> wrote: >> >> > Hello! >> > >> > I want to work on https://issues.apache.org/jira/browse/IGNITE-6879 >> issue. >> > Following the rules here >> > https://ignite.apache.org/community/contribute.html#contribute my Jira >> > username is "homich" so assign me to this ticket please. >> > >> > P.S. I found this rules page after I made PR, so sorry for this >> > inconvenience. >> > >> > Regards. >> > >> > -- Best regards, Alexey |
|
|
Just found the fix is ready - I will review it today or tomorrow.
|
|
|
Hi Alexey,
Did you find the patch is looking good and is ready to be merged? Sincerely, Dmitriy Pavlov чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin <[hidden email]>: > Just found the fix is ready - I will review it today or tomorrow. > |
|
|
Hi Dmitriy,
I took a look for PR, it needs codestyle fixes. 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: > Hi Alexey, > > Did you find the patch is looking good and is ready to be merged? > > Sincerely, > Dmitriy Pavlov > > чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin <[hidden email]>: > > > Just found the fix is ready - I will review it today or tomorrow. > > > |
|
|
HI Dmitriy, thank you!
Roman, could you please address Dmitriy's comments? чт, 22 мар. 2018 г. в 19:18, Дмитрий Рябов <[hidden email]>: > Hi Dmitriy, > > I took a look for PR, it needs codestyle fixes. > > 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: > >> Hi Alexey, >> >> Did you find the patch is looking good and is ready to be merged? >> >> Sincerely, >> Dmitriy Pavlov >> >> чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin <[hidden email] >> >: >> >> > Just found the fix is ready - I will review it today or tomorrow. >> > >> > > |
|
|
Hi all!
Dmitriy thank you for review. I`ve just fixed all your comments. чт, 22 мар. 2018 г. в 20:36, Dmitry Pavlov <[hidden email]>: > HI Dmitriy, thank you! > > Roman, could you please address Dmitriy's comments? > > чт, 22 мар. 2018 г. в 19:18, Дмитрий Рябов <[hidden email]>: > >> Hi Dmitriy, >> >> I took a look for PR, it needs codestyle fixes. >> >> 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: >> >>> Hi Alexey, >>> >>> Did you find the patch is looking good and is ready to be merged? >>> >>> Sincerely, >>> Dmitriy Pavlov >>> >>> чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin <[hidden email] >>> >: >>> >>> > Just found the fix is ready - I will review it today or tomorrow. >>> > >>> >> >> |
|
|
Dmitry, Alexey, code and tests looks good for me.
JIRA: https://issues.apache.org/jira/browse/IGNITE-6879 PR: https://github.com/apache/ignite/pull/3704 Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-541 TeamCity: https://ci.ignite.apache.org/viewLog.html?buildId=1170373&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll 2018-03-22 21:48 GMT+03:00 Роман Меерсон <[hidden email]>: > Hi all! > > Dmitriy thank you for review. > > I`ve just fixed all your comments. > > чт, 22 мар. 2018 г. в 20:36, Dmitry Pavlov <[hidden email]>: > >> HI Dmitriy, thank you! >> >> Roman, could you please address Dmitriy's comments? >> >> чт, 22 мар. 2018 г. в 19:18, Дмитрий Рябов <[hidden email]>: >> >>> Hi Dmitriy, >>> >>> I took a look for PR, it needs codestyle fixes. >>> >>> 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: >>> >>>> Hi Alexey, >>>> >>>> Did you find the patch is looking good and is ready to be merged? >>>> >>>> Sincerely, >>>> Dmitriy Pavlov >>>> >>>> чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin < >>>> [hidden email]>: >>>> >>>> > Just found the fix is ready - I will review it today or tomorrow. >>>> > >>>> >>> >>> |
|
|
Hi all!
I suppose Dmity`s message wasn`t april 1st joke) So what about my PR? Would it be merged? вс, 1 апр. 2018 г. в 12:40, Дмитрий Рябов <[hidden email]>: > Dmitry, Alexey, code and tests looks good for me. > > JIRA: https://issues.apache.org/jira/browse/IGNITE-6879 > PR: https://github.com/apache/ignite/pull/3704 > Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-541 > TeamCity: > https://ci.ignite.apache.org/viewLog.html?buildId=1170373&tab=buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll > > 2018-03-22 21:48 GMT+03:00 Роман Меерсон <[hidden email]>: > >> Hi all! >> >> Dmitriy thank you for review. >> >> I`ve just fixed all your comments. >> >> чт, 22 мар. 2018 г. в 20:36, Dmitry Pavlov <[hidden email]>: >> >>> HI Dmitriy, thank you! >>> >>> Roman, could you please address Dmitriy's comments? >>> >>> чт, 22 мар. 2018 г. в 19:18, Дмитрий Рябов <[hidden email]>: >>> >>>> Hi Dmitriy, >>>> >>>> I took a look for PR, it needs codestyle fixes. >>>> >>>> 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: >>>> >>>>> Hi Alexey, >>>>> >>>>> Did you find the patch is looking good and is ready to be merged? >>>>> >>>>> Sincerely, >>>>> Dmitriy Pavlov >>>>> >>>>> чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin < >>>>> [hidden email]>: >>>>> >>>>> > Just found the fix is ready - I will review it today or tomorrow. >>>>> > >>>>> >>>> >>>> > |
|
|
Hi!
No, it wasn't a joke) 2018-04-04 11:22 GMT+03:00 Роман Меерсон <[hidden email]>: > Hi all! > I suppose Dmity`s message wasn`t april 1st joke) > So what about my PR? Would it be merged? > > вс, 1 апр. 2018 г. в 12:40, Дмитрий Рябов <[hidden email]>: > >> Dmitry, Alexey, code and tests looks good for me. >> >> JIRA: https://issues.apache.org/jira/browse/IGNITE-6879 >> PR: https://github.com/apache/ignite/pull/3704 >> Upsource: https://reviews.ignite.apache.org/ignite/review/IGNT-CR-541 >> TeamCity: https://ci.ignite.apache.org/viewLog.html?buildId=1170373&tab= >> buildResultsDiv&buildTypeId=IgniteTests24Java8_RunAll >> >> 2018-03-22 21:48 GMT+03:00 Роман Меерсон <[hidden email]>: >> >>> Hi all! >>> >>> Dmitriy thank you for review. >>> >>> I`ve just fixed all your comments. >>> >>> чт, 22 мар. 2018 г. в 20:36, Dmitry Pavlov <[hidden email]>: >>> >>>> HI Dmitriy, thank you! >>>> >>>> Roman, could you please address Dmitriy's comments? >>>> >>>> чт, 22 мар. 2018 г. в 19:18, Дмитрий Рябов <[hidden email]>: >>>> >>>>> Hi Dmitriy, >>>>> >>>>> I took a look for PR, it needs codestyle fixes. >>>>> >>>>> 2018-03-19 14:22 GMT+03:00 Dmitry Pavlov <[hidden email]>: >>>>> >>>>>> Hi Alexey, >>>>>> >>>>>> Did you find the patch is looking good and is ready to be merged? >>>>>> >>>>>> Sincerely, >>>>>> Dmitriy Pavlov >>>>>> >>>>>> чт, 15 мар. 2018 г. в 11:19, Alexey Kukushkin < >>>>>> [hidden email]>: >>>>>> >>>>>> > Just found the fix is ready - I will review it today or tomorrow. >>>>>> > >>>>>> >>>>> >>>>> >> |
|
|
Roman, Dmitry,
I also reviewed the fix and the code looks OK to me. But the fix has significant implication - Ignite no longer can be used with spring-data 1.0 due to no backward compatibility between spring 2.0 and 1.0 APIs. With this approach we must remember to add corresponding spring-data migration instructions to the future ignite 2.5 migration guide. We could keep spring 1 support and backward compatibility by creating a new module "ignite-spring-2-data" and keeping existing ignite-spring-data as is. I do not like this option since to me increased complexity and maintainability costs overweight the benefits of protecting "Ignite-spring-1" users. I suggest you find a committer (see this list <https://ignite.apache.org/community/resources.html#people>), communicate the implication I mentioned above and say that two people already approved the code providing we are OK with the chosen approach. |
|
|
I agree that increasing complexity isn't good idea. Roman, can you document
the migration guide? 2018-04-04 13:41 GMT+03:00 Alexey Kukushkin <[hidden email]>: > Roman, Dmitry, > > I also reviewed the fix and the code looks OK to me. But the fix has > significant implication - Ignite no longer can be used with spring-data 1.0 > due to no backward compatibility between spring 2.0 and 1.0 APIs. With this > approach we must remember to add corresponding spring-data migration > instructions to the future ignite 2.5 migration guide. > > We could keep spring 1 support and backward compatibility by creating a new > module "ignite-spring-2-data" and keeping existing ignite-spring-data as > is. I do not like this option since to me increased complexity and > maintainability costs overweight the benefits of protecting > "Ignite-spring-1" users. > > I suggest you find a committer (see this list > <https://ignite.apache.org/community/resources.html#people>), communicate > the implication I mentioned above and say that two people already approved > the code providing we are OK with the chosen approach. > |
|
|
Hi Igniters,
I am going to review these changes in 3-4 days. If everything is ok and if there is no objections, I will merge it. Hi Denis, are you agree with proposed change? Sincerely, Dmitriy Pavlov ср, 4 апр. 2018 г. в 14:26, Дмитрий Рябов <[hidden email]>: > I agree that increasing complexity isn't good idea. Roman, can you document > the migration guide? > > 2018-04-04 13:41 GMT+03:00 Alexey Kukushkin <[hidden email]>: > > > Roman, Dmitry, > > > > I also reviewed the fix and the code looks OK to me. But the fix has > > significant implication - Ignite no longer can be used with spring-data > 1.0 > > due to no backward compatibility between spring 2.0 and 1.0 APIs. With > this > > approach we must remember to add corresponding spring-data migration > > instructions to the future ignite 2.5 migration guide. > > > > We could keep spring 1 support and backward compatibility by creating a > new > > module "ignite-spring-2-data" and keeping existing ignite-spring-data as > > is. I do not like this option since to me increased complexity and > > maintainability costs overweight the benefits of protecting > > "Ignite-spring-1" users. > > > > I suggest you find a committer (see this list > > <https://ignite.apache.org/community/resources.html#people>), > communicate > > the implication I mentioned above and say that two people already > approved > > the code providing we are OK with the chosen approach. > > > |
|
|
Hi guys,
According to ASF stats our spring data integration is 2 times more popular than spark integraion - 900 maven downloads vs. 400. So I would suggest us creating ignite-spring-data-2.0 module to support new deployments and leave ignite-spring-data to not break existing ones. -- Denis On Wed, Apr 4, 2018 at 7:33 AM, Dmitry Pavlov <[hidden email]> wrote: > Hi Igniters, > > I am going to review these changes in 3-4 days. If everything is ok and if > there is no objections, I will merge it. > > Hi Denis, > > are you agree with proposed change? > > Sincerely, > Dmitriy Pavlov > > ср, 4 апр. 2018 г. в 14:26, Дмитрий Рябов <[hidden email]>: > > > I agree that increasing complexity isn't good idea. Roman, can you > document > > the migration guide? > > > > 2018-04-04 13:41 GMT+03:00 Alexey Kukushkin <[hidden email]>: > > > > > Roman, Dmitry, > > > > > > I also reviewed the fix and the code looks OK to me. But the fix has > > > significant implication - Ignite no longer can be used with spring-data > > 1.0 > > > due to no backward compatibility between spring 2.0 and 1.0 APIs. With > > this > > > approach we must remember to add corresponding spring-data migration > > > instructions to the future ignite 2.5 migration guide. > > > > > > We could keep spring 1 support and backward compatibility by creating a > > new > > > module "ignite-spring-2-data" and keeping existing ignite-spring-data > as > > > is. I do not like this option since to me increased complexity and > > > maintainability costs overweight the benefits of protecting > > > "Ignite-spring-1" users. > > > > > > I suggest you find a committer (see this list > > > <https://ignite.apache.org/community/resources.html#people>), > > communicate > > > the implication I mentioned above and say that two people already > > approved > > > the code providing we are OK with the chosen approach. > > > > > > |
|
|
Ok guys, I`ll made changes but what should we do with examples and with
Spring module version? Exemples couldn`t support both versions, so should i leave upgraded to 2.0 version? Spring module was upgraded to newest version, so should i leave it on newest version? чт, 5 апр. 2018 г. в 0:17, Denis Magda <[hidden email]>: > Hi guys, > > According to ASF stats our spring data integration is 2 times more popular > than spark integraion - 900 maven downloads vs. 400. > > So I would suggest us creating ignite-spring-data-2.0 module to support new > deployments and leave ignite-spring-data to not break existing ones. > > -- > Denis > > > > On Wed, Apr 4, 2018 at 7:33 AM, Dmitry Pavlov <[hidden email]> > wrote: > > > Hi Igniters, > > > > I am going to review these changes in 3-4 days. If everything is ok and > if > > there is no objections, I will merge it. > > > > Hi Denis, > > > > are you agree with proposed change? > > > > Sincerely, > > Dmitriy Pavlov > > > > ср, 4 апр. 2018 г. в 14:26, Дмитрий Рябов <[hidden email]>: > > > > > I agree that increasing complexity isn't good idea. Roman, can you > > document > > > the migration guide? > > > > > > 2018-04-04 13:41 GMT+03:00 Alexey Kukushkin <[hidden email] > >: > > > > > > > Roman, Dmitry, > > > > > > > > I also reviewed the fix and the code looks OK to me. But the fix has > > > > significant implication - Ignite no longer can be used with > spring-data > > > 1.0 > > > > due to no backward compatibility between spring 2.0 and 1.0 APIs. > With > > > this > > > > approach we must remember to add corresponding spring-data migration > > > > instructions to the future ignite 2.5 migration guide. > > > > > > > > We could keep spring 1 support and backward compatibility by > creating a > > > new > > > > module "ignite-spring-2-data" and keeping existing ignite-spring-data > > as > > > > is. I do not like this option since to me increased complexity and > > > > maintainability costs overweight the benefits of protecting > > > > "Ignite-spring-1" users. > > > > > > > > I suggest you find a committer (see this list > > > > <https://ignite.apache.org/community/resources.html#people>), > > > communicate > > > > the implication I mentioned above and say that two people already > > > approved > > > > the code providing we are OK with the chosen approach. > > > > > > > > > > |
|
|
I`ve finished with new module:
https://github.com/apache/ignite/pull/3704 чт, 5 апр. 2018 г. в 12:06, Роман Меерсон <[hidden email]>: > Ok guys, I`ll made changes but what should we do with examples and with > Spring module version? > > Exemples couldn`t support both versions, so should i leave upgraded to 2.0 > version? > Spring module was upgraded to newest version, so should i leave it on > newest version? > > чт, 5 апр. 2018 г. в 0:17, Denis Magda <[hidden email]>: > >> Hi guys, >> >> According to ASF stats our spring data integration is 2 times more popular >> than spark integraion - 900 maven downloads vs. 400. >> >> So I would suggest us creating ignite-spring-data-2.0 module to support >> new >> deployments and leave ignite-spring-data to not break existing ones. >> >> -- >> Denis >> >> >> >> On Wed, Apr 4, 2018 at 7:33 AM, Dmitry Pavlov <[hidden email]> >> wrote: >> >> > Hi Igniters, >> > >> > I am going to review these changes in 3-4 days. If everything is ok and >> if >> > there is no objections, I will merge it. >> > >> > Hi Denis, >> > >> > are you agree with proposed change? >> > >> > Sincerely, >> > Dmitriy Pavlov >> > >> > ср, 4 апр. 2018 г. в 14:26, Дмитрий Рябов <[hidden email]>: >> > >> > > I agree that increasing complexity isn't good idea. Roman, can you >> > document >> > > the migration guide? >> > > >> > > 2018-04-04 13:41 GMT+03:00 Alexey Kukushkin < >> [hidden email]>: >> > > >> > > > Roman, Dmitry, >> > > > >> > > > I also reviewed the fix and the code looks OK to me. But the fix has >> > > > significant implication - Ignite no longer can be used with >> spring-data >> > > 1.0 >> > > > due to no backward compatibility between spring 2.0 and 1.0 APIs. >> With >> > > this >> > > > approach we must remember to add corresponding spring-data migration >> > > > instructions to the future ignite 2.5 migration guide. >> > > > >> > > > We could keep spring 1 support and backward compatibility by >> creating a >> > > new >> > > > module "ignite-spring-2-data" and keeping existing >> ignite-spring-data >> > as >> > > > is. I do not like this option since to me increased complexity and >> > > > maintainability costs overweight the benefits of protecting >> > > > "Ignite-spring-1" users. >> > > > >> > > > I suggest you find a committer (see this list >> > > > <https://ignite.apache.org/community/resources.html#people>), >> > > communicate >> > > > the implication I mentioned above and say that two people already >> > > approved >> > > > the code providing we are OK with the chosen approach. >> > > > >> > > >> > >> > |
|
|
Roman,
Just two small comments from me: 1. I suggest renaming IgniteSpringDataTestSuite to IgniteSpringData2TestSuite: we must be able to test both spring-data and spring-data-2 JARs with single "mvn test" 2. Make sure "Ignite Spring Data" test job in TeamCity is extended to run the new test suite after the code is committed. Other than that the new module looks OK to me. |
|
|
Alexey,
1) Fixed 2) How could i be sure? What do i need to do? чт, 5 апр. 2018 г. в 14:24, Alexey Kukushkin <[hidden email]>: > Roman, > > Just two small comments from me: > > 1. I suggest renaming IgniteSpringDataTestSuite to > IgniteSpringData2TestSuite: we must be able to test both spring-data and > spring-data-2 JARs with single "mvn test" > 2. Make sure "Ignite Spring Data" test job in TeamCity is extended to > run the new test suite after the code is committed. > > Other than that the new module looks OK to me. > |
«
Return to Apache Ignite Developers - Legacy Mail Archive
|
1 view|%1 views
| Free forum by Nabble | Edit this page |