Breaking change: Remove expand(_forward_) and expand(_reverse_). #4119
Conversation
martinmr
requested review from
danielmai,
manishrjain,
MichaelJCompton,
pawanrawal and
a team
as code owners
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@martinmr you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
| 1 Message | |
|---|---|
| ✅ |
|
danielmai
changed the title
danielmai
changed the title
| @@ -1824,7 +1824,7 @@ func expandSubgraph(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { | |||
| } | |||
|
|
|||
| switch child.Params.Expand { | |||
| // It could be expand(_all_), expand(_forward_), expand(_reverse_) or expand(val(x)). | |||
| // It could be expand(_all_) or expand(val(x)). | |||
There was a problem hiding this comment.
What's val(x)? I have never any example like this
There was a problem hiding this comment.
This was a variable that gets predicates values from the old _predicate_ func. To expand a query with those preds. It was like "expand only the preds found in the _predicate_ func".
There was a problem hiding this comment.
Pasted from my reply in Reviewable: "It takes a variable (a list of string values) and uses those values as the names of the predicates to expand. We should discuss whether to keep it but I left it untouched for now."
There was a problem hiding this comment.
Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @campoy, @danielmai, @manishrjain, @MichaelJCompton, and @pawanrawal)
query/query.go, line 1827 at r1 (raw file):
Previously, campoy (Francesc Campoy) wrote…
What's val(x)? I have never any example like this
It takes a variable (a list of string values) and uses those values as the names of the predicates to expand. We should discuss whether to keep it but I left it untouched for now.
wiki/content/query-language/index.md, line 1800 at r1 (raw file):
Previously, campoy (Francesc Campoy) wrote…
Since we're at it, this statement is not accurate.
Done.
There was a problem hiding this comment.
Reviewed 4 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, @manishrjain, @martinmr, and @MichaelJCompton)
gql/parser.go, line 2805 at r2 (raw file):
case "_all_": child.Expand = "_all_" case "_forward_":
Should have a custom error message for _forward_ and _reverse_, saying that these are not supported anymore and the user should use _all_?
There was a problem hiding this comment.
Just a reminder to make sure the basis of dropping support for these options is communicated to those who may be using forward or reverse.
Reviewed with ❤️ by PullRequest
There was a problem hiding this comment.
Reviewable status: 3 of 5 files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, @manishrjain, @MichaelJCompton, and @pawanrawal)
gql/parser.go, line 2805 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Should have a custom error message for
_forward_and_reverse_, saying that these are not supported anymore and the user should use_all_?
Done. The message does not include anything about _all_ because it's not a direct replacement.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @campoy, @danielmai, @manishrjain, @MichaelJCompton, and @pawanrawal)
This change is