Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct limit processing in query batch iterator #9621

Closed
wants to merge 1 commit into from
Closed

Correct limit processing in query batch iterator #9621

wants to merge 1 commit into from

Conversation

flancer64
Copy link
Contributor

@flancer64 flancer64 commented May 13, 2017

#8018
#7968

We should increase LIMIT to collect all records with ID=$max that are over the limit in selection.

@okorshenko okorshenko self-assigned this Jul 17, 2017
@okorshenko okorshenko added this to the July 2017 milestone Jul 17, 2017
@okorshenko
Copy link
Contributor

Hi @flancer64
Could you please provide more information about this PR following the PR description standard?

Also, could you please fix broken tests and cover your changes with integration tests.

Thank you

@flancer64
Copy link
Contributor Author

Hello, guys.

No, I could not. I had solved my own problem - missed products on the front in the Magento 2 store. I have shared my solution as this PR and as standalone module. I can answer your questions if you will ask me. But I will not fix your unit tests and create integration test. This is your project and this is your responsibility. You should analyze this solution (it might be wrong fix and the unit tests errors would say about it), you should fix your unit tests if solution is acceptable (or you can offer a better solution based on this one) and you should create your integration test (I know nothing about your integration tests) that integrationally tests this case in the suite of the other similar cases (which cases? I don't know). This is your job, not mine. I can help you to do it, but I will not do it for you.

Sincerely yours,

Alex Gusev.

@okorshenko
Copy link
Contributor

Hi @flancer64 Thank you for the quick response. Unfortunately, we can not accept this PR as is without tests. We will consider your solution for this issue. Thank you for your contribution.
Closing this PR.

@okorshenko okorshenko closed this Jul 19, 2017
@flancer64 flancer64 deleted the patch-1 branch July 19, 2017 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants