Skip to content

Conversation

david-crespo
Copy link
Contributor

@david-crespo david-crespo commented Mar 20, 2025

Extracted from #7339 for use in #7277. This PR does not use the pagination helper in any endpoints. There are proper integration tests like test_audit_log_list in #7339 demonstrating the ordering and cursor work as expected.

error.to_string(),
"unknown variant `descending`, expected `ascending`"
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests are all kinda meh to me but they run instantly and it was easy to follow the pattern of the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I take it back, they caught a mistake I made.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Lovely, thank you! I noticed one stale comment, but besides that, looks good to me! Perhaps @davepacheco or someone else will also want to review, since this is dropshot related, but I think it seems pretty uncontroversial.

@hawkw
Copy link
Member

hawkw commented Mar 20, 2025

The buildomat failures don't look like actual test failures, I think something just got cancelled: https://buildomat.eng.oxide.computer/wg/0/details/01JPT598J4PT2MQZK7WA9C3PY9/BxgIch0lAi33zswBET6w88lsqbcYvrtHvBGGUkzfZznj6Y4T/01JPT59SZ3Y8G7741P4BBSJNDD#S4692

@david-crespo
Copy link
Contributor Author

I manually cancel the old jobs when I force push so they don't take up queue space for no reason.

@david-crespo david-crespo enabled auto-merge (squash) March 20, 2025 16:42
@david-crespo david-crespo merged commit df35ee9 into main Mar 20, 2025
16 checks passed
@david-crespo david-crespo deleted the pag-time-id branch March 20, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants