Skip to content

clarify batch aggregation scrubbing - #4911

Merged
tgeoghegan merged 1 commit into
mainfrom
timg/clarify-batch-aggregation-scrubbing
Oct 20, 2026
Merged

tgeoghegan merged 1 commit into
mainfrom
timg/clarify-batch-aggregation-scrubbing

Conversation

@tgeoghegan

@tgeoghegan tgeoghegan commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Adds some doccomments on items related to scrubbed batch aggregations, as well as tidying up comments in the DB schema so they don't scroll off the right side of one's editor window.

@tgeoghegan tgeoghegan added the allow-changed-migrations Override the ci-migrations check to allow migrations that have changed. label Oct 14, 2026
@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch 3 times, most recently from f14dbe9 to c7ea143 Compare October 19, 2026 18:18
@tgeoghegan
tgeoghegan changed the base branch from main to timg/leader-selected-collection-job-idempotency-key-collector-integration-interop October 19, 2026 18:18
@tgeoghegan
tgeoghegan added this pull request to stack #4914 October 19, 2026 18:18
@tgeoghegan
tgeoghegan marked this pull request as ready for review October 19, 2026 18:18
@tgeoghegan
tgeoghegan requested a review from a team as a code owner October 19, 2026 18:18
Comment thread aggregator_core/src/datastore/models.rs Outdated
// ReportAggregationStateCode exists alongside the public ReportAggregationState because there is no
// apparent way to denote a Postgres enum literal without deriving FromSql/ToSql on a Rust enum
// type, but it is not possible to derive FromSql/ToSql on a non-C-style enum.
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.
/// BatchAggregationStateCode corresponds to the `batch_aggregation_state` enum.

Comment thread aggregator_core/src/datastore/models.rs Outdated
// type, but it is not possible to derive FromSql/ToSql on a non-C-style enum.
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.
///
/// It exists alongside the public ReportAggregationState because there is no apparent way to denote

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// It exists alongside the public ReportAggregationState because there is no apparent way to denote
/// It exists alongside the public BatchAggregationState because there is no apparent way to denote

@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch 2 times, most recently from 99974f5 to 5d40a92 Compare October 20, 2026 14:51
Base automatically changed from timg/leader-selected-collection-job-idempotency-key-collector-integration-interop to main October 20, 2026 15:11
@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch from 5d40a92 to 672ed30 Compare October 20, 2026 15:11
Adds some doccomments on items related to scrubbed batch aggregations,
as well as tidying up comments in the DB schema so they don't scroll off
the right side of one's editor window.
@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch from 672ed30 to 206e99c Compare October 20, 2026 15:25
@tgeoghegan
tgeoghegan merged commit 534b341 into main Oct 20, 2026
18 of 22 checks passed
@tgeoghegan
tgeoghegan deleted the timg/clarify-batch-aggregation-scrubbing branch October 20, 2026 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-changed-migrations Override the ci-migrations check to allow migrations that have changed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants