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.

Sign up for GitHub

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

Jump to bottom

Expanded sequence sorting #876

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Expanded sequence sorting #876

merged 7 commits into from
Apr 24, 2023

Conversation

cohansen
Copy link
Contributor

@cohansen cohansen commented Apr 18, 2023 •
edited by camargo
Loading

Closes #866
Closes #879

  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

Adds a new timeSorted flag to the sequence metadata and adds sorting if every command is EPOCH_RELATIVE.

Verification

Updated existing tests and added new tests.

Documentation

NA

Future work

Possibly more sorting behavior? Hopefully this catches all of the valid cases.

@cohansen cohansen requested a review from goetzrrGit April 18, 2023 16:40
@cohansen cohansen requested a review from a team as a code owner April 18, 2023 16:40
@cohansen cohansen self-assigned this Apr 18, 2023
@cohansen cohansen requested a review from JoelCourtney April 18, 2023 16:40
@cohansen cohansen temporarily deployed to e2e-test April 18, 2023 16:40 — with GitHub Actions Inactive
@camargo camargo added feature A new feature or feature request sequencing Anything related to the sequencing domain labels Apr 18, 2023
@camargo camargo changed the title Feature/expanded sequence sorting Expanded sequence sorting Apr 18, 2023
@cohansen cohansen mentioned this pull request Apr 18, 2023
Closed
@cohansen cohansen force-pushed the feature/expanded-sequence-sorting branch from 9df8cfe to 7708fd6 Compare April 18, 2023 17:35
@cohansen cohansen temporarily deployed to e2e-test April 18, 2023 17:35 — with GitHub Actions Inactive
camargo
camargo approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@camargo camargo left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM

@cohansen cohansen force-pushed the feature/expanded-sequence-sorting branch from 7708fd6 to 516e4dc Compare April 20, 2023 15:00
@cohansen cohansen temporarily deployed to e2e-test April 20, 2023 15:00 — with GitHub Actions Inactive
@cohansen cohansen temporarily deployed to e2e-test April 24, 2023 17:58 — with GitHub Actions Inactive
@cohansen cohansen temporarily deployed to e2e-test April 24, 2023 20:14 — with GitHub Actions Inactive
cartermak
cartermak approved these changes Apr 24, 2023
View reviewed changes
Copy link
Member

@cartermak cartermak left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for taking the time to workshop the details!

@camargo
Copy link
Member

camargo commented Apr 24, 2023

Thanks so much for taking a look @cartermak, it's super helpful

@cohansen cohansen force-pushed the feature/expanded-sequence-sorting branch from 12bd491 to 91eb228 Compare April 24, 2023 20:30
@cohansen cohansen temporarily deployed to e2e-test April 24, 2023 20:31 — with GitHub Actions Inactive
@cohansen cohansen merged commit 299db31 into develop Apr 24, 2023
@cohansen cohansen deleted the feature/expanded-sequence-sorting branch April 24, 2023 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cartermak cartermak cartermak approved these changes

@camargo camargo camargo approved these changes

@goetzrrGit goetzrrGit Awaiting requested review from goetzrrGit

@JoelCourtney JoelCourtney Awaiting requested review from JoelCourtney JoelCourtney is a code owner automatically assigned from NASA-AMMOS/aerie-backend

Assignees

@cohansen cohansen

Labels
feature A new feature or feature request sequencing Anything related to the sequencing domain
Projects
None yet
Milestone
No milestone
3 participants
@cohansen @camargo @cartermak