Skip to content

Conversation

@rosahbruno
Copy link
Contributor

Checklist for reviewer:

For glean changes:

  • Update templates/include/glean/CHANGELOG.md

For modifications to schemas in restricted namespaces (see CODEOWNERS):

@rosahbruno rosahbruno force-pushed the 1862955-gleanjs-sessions branch from 82d9ff3 to b27a064 Compare December 18, 2023 20:25
@rosahbruno rosahbruno marked this pull request as ready for review December 18, 2023 20:33
@auto-assign auto-assign bot requested a review from relud December 18, 2023 20:33
@relud relud removed their request for review December 19, 2023 19:01
@rosahbruno
Copy link
Contributor Author

@akkomar
Would you be able to review this?

@akkomar akkomar self-requested a review February 5, 2024 18:10
Copy link
Contributor

@akkomar akkomar left a comment

Choose a reason for hiding this comment

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

LGTM. Since I haven't seen the fields that are added here documented in bugs or in the doc, can you re-request the review when mozilla/glean.js#1850 is finalized?

@whd this will add two fields to all Glean tables. Do you want a heads-up before merge?

Copy link
Member

@whd whd left a comment

Choose a reason for hiding this comment

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

@whd this will add two fields to all Glean tables. Do you want a heads-up before merge?

No unless you want to manually verify the efficacy of this PR in stage without automatically propagating to prod. Scanning history I can't recall being involved in the previous changes to these schemas. I'm mostly interested in PB[ER] changes since those can require an ingestion-sink code update.

@rosahbruno rosahbruno requested a review from akkomar March 1, 2024 19:13
@rosahbruno
Copy link
Contributor Author

@akkomar
The Glean.js PRs have landed and I added the missing CHANGELOG entry that @whd pointed out. This should be ready for review again.

Copy link
Contributor

@akkomar akkomar left a comment

Choose a reason for hiding this comment

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

@rosahbruno rosahbruno force-pushed the 1862955-gleanjs-sessions branch from 4e22387 to ed0a2d4 Compare March 4, 2024 17:34
@akkomar akkomar merged commit b3dad47 into mozilla-services:main Mar 4, 2024
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.

3 participants