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

TINKERPOP-3130 Allowed discard() as anonymous and chained #3024

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spmallette
Copy link
Contributor

https://issues.apache.org/jira/browse/TINKERPOP-3130

Should be a non-breaking change, in that it just expands what discard() can do to be more consistent in its usage.

VOTE +1

@spmallette spmallette changed the title TINKERPOP-3130 Allowed none() as anonymous and chained TINKERPOP-3130 Allowed discard() as anonymous and chained Feb 7, 2025
@spmallette spmallette force-pushed the TINKERPOP-3130-master branch from 0479f54 to 65337de Compare February 7, 2025 14:48
@@ -1469,6 +1473,10 @@ def min_(cls, *args):
def none(cls, *args):
return cls.graph_traversal(None, None, GremlinLang()).none(*args)

@classmethod
def none(cls, *args):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is already defined above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a copy error that should be removed, it's also adding Bytecode().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed that merge issue during the cherry-pick - good catch.


Scenario: g_V_discard_fold_none
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be named discard_fold_discard?

When iterated to list
Then the result should be empty

Scenario: g_V_discard_none
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be discard_discard?

@kenhuuu
Copy link
Contributor

kenhuuu commented Feb 7, 2025

VOTE +1, pending closure of comments made by other reviewers.

@@ -470,6 +470,14 @@ public static GraphTraversal<object, object> Difference(object differenceObject)
return new GraphTraversal<object, object>().Difference(differenceObject);
}

/// <summary>
/// Spawns a <see cref="GraphTraversal{SType, EType}" /> and adds the none step to that traversal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit on docs

Suggested change
/// Spawns a <see cref="GraphTraversal{SType, EType}" /> and adds the none step to that traversal.
/// Spawns a <see cref="GraphTraversal{SType, EType}" /> and adds the discard step to that traversal.

@xiazcy
Copy link
Contributor

xiazcy commented Feb 7, 2025

VOTE +1 pending comment resolution.

@spmallette spmallette force-pushed the TINKERPOP-3130-master branch from 65337de to 6a52c2f Compare February 8, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants