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

ResultSet.hasNext should be idempotent #346

Closed
wants to merge 2 commits into from

Conversation

robertdale
Copy link
Member

Sorry I didn't create a ticket; JIRA is having problems.

Calling hasNext() multiple times would dequeue items.

@spmallette
Copy link
Contributor

This looks like a good change. Thanks for this. Did you run the Gremlin Server integration tests after making this change?

@robertdale
Copy link
Member Author

Yes, I ran 'mvn clean install -DskipIntegrationTests=false -DincludeNeo4j' and it passed.

@spmallette
Copy link
Contributor

VOTE +1

@dkuppitz
Copy link
Contributor

VOTE: +1

@spmallette
Copy link
Contributor

@robertdale when we get the final vote for this PR to be merged, i expect to merge it to tp31 and then master as I'd like the 3.1.x line of code to get this fix. i only mention this because i see that you now have what looks like an empty merge commit on this PR at this point. Do you know what that is? Is that something you can clean up from your end?

note to the final voter - i will update changelog on merge.

@robertdale
Copy link
Member Author

I merged your master into my fork. Commit 0787e46
I didn't realize it would affect the pull request. Next time I'll create a branch.

@spmallette
Copy link
Contributor

You could just use git rebase to update your branch rather than git merge - in that way the commit history doesn't get weird.

@robertdale
Copy link
Member Author

Normally that's what I would do but tried doing it the 'github way' - https://help.github.com/articles/syncing-a-fork/

So let me know what you want me to do now. I can start over, creating the JIRA ticket, branch, etc.

@spmallette
Copy link
Contributor

no need to start over - maybe not an issue. i'll see what happens when i merge. thanks

@okram
Copy link
Contributor

okram commented Jul 6, 2016

Smart. VOTE +1.

@asfgit asfgit closed this in 96cf3f2 Jul 6, 2016
@robertdale robertdale deleted the master branch July 6, 2016 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants