Explicitly specify default ordering of segments listing#223
Merged
Conversation
samdbmg
approved these changes
Jul 3, 2026
|
|
||
| It may also use a subset of the samples in the Media Object, and if the `include_object_timerange=true` parameter is set, the object's timerange will also be returned to aid identifying which samples to skip. | ||
|
|
||
| Segments are sorted by ascending timerange by default. |
Member
There was a problem hiding this comment.
Nit: Does this read better?
Suggested change
| Segments are sorted by ascending timerange by default. | |
| Segments are sorted by timerange, in ascending order by default. |
Also do we need a reminder that it doesn't matter whether you sort by timerange start or end: the result is the same because they cannot overlap?
Contributor
Author
There was a problem hiding this comment.
I was thinking that. I couldn't decide if it was stating the obvious or not.
Contributor
Author
There was a problem hiding this comment.
Thinking about this over the weekend, I've added in a note on the sort key. Further explanation isn't a bad thing.
Contributor
Author
There was a problem hiding this comment.
I have also included a note about edge cases involving clusivity markers.
Co-authored-by: Sam Mesterton-Gibbons <sam.mesterton-gibbons@bbc.co.uk>
j616
force-pushed
the
jamessa-segmentSort
branch
2 times, most recently
from
17c5873 to
fc660f2
Compare
samdbmg
approved these changes
Jul 8, 2026
samdbmg
left a comment
samdbmg
left a comment
Member
There was a problem hiding this comment.
LGTM - I agree on your point about more explanation - and good thought on edge cases
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Explicitly specify default ordering of segments listing
Issue (if relevant)
GitHub Issue: #219
Related PRs
Submitter PR Checks
Reviewer PR Checks
Info on PRs
The checks above are guidelines. They don't all have to be ticked, but they should all have been considered.