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

Move descendant selector section #228

Closed glyn closed 2 years ago

glyn commented 2 years ago

I want to take the opportunity of there being no PRs in flight to make this purely editorial change to the order of the spec.

It avoids unnecessary forward references to the filter and list selectors.

gregsdennis commented 2 years ago

Another PR opened and merged in less than 8 hours...

glyn commented 2 years ago

Another PR opened and merged in less than 8 hours...

Please note this PR was purely editorial. It moved some text around. but didn't change it. (I was planning to merge this later today after a bit more time.)

gregsdennis commented 2 years ago

We should just have a flat policy of 24 hrs for a PR to ensure everyone gets to see it. It doesn't matter if it was a spelling change. Everyone should have a chance to review it.

glyn commented 2 years ago

It's one thing to agree to this policy, which I think we have, and quite another to remember to apply it when one is focussed on other things. I've raised a question on stack overflow about automating this check.

cabo commented 2 years ago

It's one thing to agree to this policy, which I think we have,

Have we? It is not policy I would normally agree to.

Editors need to be able to do their work. Github is here to help, not to hinder.

glyn commented 2 years ago

Ah, my mistake: I agreed.

gregsdennis commented 2 years ago

We have a two-week policy for PRs in JSON Schema and it's very helpful to encourage cooperation. I don't think 24 hrs is too much to ask.

cabo commented 2 years ago

Please explain how you did not get a chance to review this change.

gregsdennis commented 2 years ago

Please explain how you did not get a chance to review this change.

The PR opened at 3AM and merged at 10AM. This is an insufficient window. 24 hrs, please. It's not a big request.

cabo commented 2 years ago

You have had time to review after merge. How is a merge such an important event?

gregsdennis commented 2 years ago

If I have objections to a PR, they should be voiced before it's merged. Reviewing after the fact is pointless. Any damage has been done.

This particular PR is not one of those cases, but in general, everyone involved should get a chance to review.

Why are you so opposed to this? It's one day. That's all I'm asking.