search

ietf-wg-jsonpath / draft-ietf-jsonpath-base

Development of a JSONPath internet draft
https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/
Other
59 stars 20 forks source link

Allow wildcard in list selector #255

Closed glyn closed 1 year ago

glyn commented 2 years ago

https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/201#issuecomment-1231469606 suggested that, in current draft terminology, a list selector should include wildcards. Even though this isn't particularly useful, it is in the interest of both ease of implementation and to keep the specification more concise.

gregsdennis commented 2 years ago

I'm open to this, but can we hold this until #201 is resolved, please? I'm really trying to ensure there are no functional changes with it, and merging functional changes while I'm writing a PR are hard to track. I fear something will be missed.

glyn commented 2 years ago

I think we need to agree the process in a bit more detail.

I thought we were identifying technical changes in #210, raising those as separate issues/PRs, and merging them before embarking on a purely editorial PR to cover the rest of #201.

Otherwise, a PR covering #201 and avoiding technical changes won't get the benefit of the changes we are identifying.

Precisely who creates the editorial PR is to be decided. I'd be happy to work on that once we've settled the outstanding questions. If I work on it, I will try to construct it as a series of incremental commits to make it easy to review.

With that in mind, @gregsdennis's branch can be considered a prototype which we can use to help us agree the direction of #201.

Is that process acceptable?

Assuming it is, there is then the question of whether #255 (and this PR) is a technical change we want to adopt. @cabo proposed it and I agreed it was an improvement. I'd be interested in what others have to say.

gregsdennis commented 2 years ago

I thought we were identifying technical changes in #210, raising those as separate issues.

Yes.

/PRs, and merging them before embarking on a purely editorial PR to cover the rest of #201.

No. I'd like us to stay focused on the editorial change for now. We can keep track of functional changes in issues like this one, but trying to track functional changes that are happening while attempting to make purely editorial change will inevitably lead to a mistake. This can be avoided by focusing on one thing at a time.

Precisely who creates the editorial PR is to be decided.

This is my proposal, and I'd like to do the PR to make sure it's in line with what I have in mind. Additionally, I've already done the legwork to identify a target. All that remains is massaging the commits.

Assuming it is, there is then the question of whether #255 (and this PR) is a technical change we want to adopt. @cabo proposed it and I agreed it was an improvement. I'd be interested in what others have to say.

I have no disagreement that this is something that we want to change. I just prefer not to change it until the editorial is complete.

glyn commented 2 years ago

This is my proposal, and I'd like to do the PR to make sure it's in line with what I have in mind. Additionally, I've already done the legwork to identify a target. All that remains is massaging the commits.

Ok, as I'm AFK for a week or so, I'd be comfortable having a moratorium on merging PRs so you can complete your editorial PR for #201.

For the record, I'm still concerned about the direction of #201 and that probably manifests itself most obviously in the semantic interface between a *appender and its *pickers (and the result ordering aspect). If you prefer to address this in the PR rather than ahead of time, that's fine, but there is a risk of increased rework if that semantic interface isn't as clean as you expect.

cabo commented 1 year ago

Interim 2022-09-27: Decided that we go for allowing wildcard *pickers everywhere pickers are allowed. To be covered by #258.