V3 reactor - Githubissues

microsoft / spring-data-cosmosdb

Access data with Azure Cosmos DB

MIT License

94 stars 64 forks source link

V3 reactor #370

Closed mbhaskar closed 5 years ago

mbhaskar commented 5 years ago

Adding support to Reactor types using Cosmos V3 beta SDK This PR includes Reactive Operations, Templates and Repository as part of a complete plan of migrating entire operations to new Cosmos SDK

Incarnation-p-lee commented 5 years ago

@mbhaskar The patch looks cool but seems a little big here. So I would like to split this patch in some small ones.

Add dependency and configuration files like Factory, FactoryBean.
Add repository interface and implementation with placeholder, like simpleRepository implement throw NotImplemented for now.
Implemented all method one by one with fully tested code, you can choose one or more method of interface by the code change lines.

How do you think of it ?

christopheranderson commented 5 years ago

@Incarnation-p-lee That's a fair amount of work that wasn't discussed up front. I'd prefer that we review this PR as is and in the future we'll try to meet that standard. By the very nature of duplicating the functionality of the existing plugin, this was going to be lots of changes. Individual commits which add non-functional APIs seems like over engineering and we have folks waiting for this to be released...

Please review this as is.

Incarnation-p-lee commented 5 years ago

I am afraid that is what we discssed from last meeting. In fact it will faster the whole process in some small PRs instead of large PR with more than 1000 lines changes. Yes, we may accept PR like this if and only if it cannot be separated. But this one should be separated in 3-4 PR. As production project, no matter our repo or yours, one PR with thousands of lines may not be the best practice. From my personally experience, small PRs with same feature may saving more time. How do u think of it?