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.

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

Bug 1811857, bug 1811888 - Fully remove support for the old events API #2346

Merged
merged 3 commits into from
Jan 25, 2023

Conversation

badboy
Copy link
Member

@badboy badboy commented Jan 23, 2023

Will require a glean_parser change.

Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

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

This seems worthy of a changelog entry noting the change from "deprecated" -> "removed", but other than that LGTM!

@badboy badboy changed the title Fully remove support for the old events API Bug 1811857 - Fully remove support for the old events API Jan 23, 2023
@badboy
Copy link
Member Author

badboy commented Jan 23, 2023

This seems worthy of a changelog entry noting the change from "deprecated" -> "removed", but other than that LGTM!

Way ahead of you: https://github.com/mozilla/glean/blob/main/CHANGELOG.md#v5200-2022-12-13
The API has been removed in Glean v52. This just removes the internals for it.

@badboy badboy changed the title Bug 1811857 - Fully remove support for the old events API Bug 1811857, bug 1811888 - Fully remove support for the old events API Jan 24, 2023
@badboy badboy force-pushed the completely-remove-old-eventextras-api branch from f3c7b2f to 67029d1 Compare January 24, 2023 12:24
@badboy badboy marked this pull request as ready for review January 24, 2023 12:24
@badboy badboy requested a review from a team as a code owner January 24, 2023 12:24
@badboy badboy requested review from rosahbruno and removed request for a team January 24, 2023 12:24
@badboy badboy force-pushed the completely-remove-old-eventextras-api branch from 67029d1 to 53a5d12 Compare January 24, 2023 12:29
@badboy
Copy link
Member Author

badboy commented Jan 24, 2023

Note that all other changes except the glean_parser update itself aren't visible to users so I didn't put them in the changelog

@badboy badboy force-pushed the completely-remove-old-eventextras-api branch from 53a5d12 to c08ca0f Compare January 24, 2023 12:40
@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Base: 29.41% // Head: 29.41% // No change to project coverage 👍

Coverage data is based on head (c08ca0f) compared to base (cc6316b).
Patch has no changes to coverable lines.

❗ Current head c08ca0f differs from pull request most recent head 2484e5d. Consider uploading reports for the commit 2484e5d to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2346   +/-   ##
=======================================
  Coverage   29.41%   29.41%           
=======================================
  Files           1        1           
  Lines          34       34           
=======================================
  Hits           10       10           
  Misses         24       24           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@badboy badboy requested review from travis79 and removed request for rosahbruno January 24, 2023 14:18
@badboy badboy force-pushed the completely-remove-old-eventextras-api branch from c08ca0f to 2484e5d Compare January 24, 2023 15:36
Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

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

Still looks good to me!

@badboy badboy merged commit 50c22fc into main Jan 25, 2023
@badboy badboy deleted the completely-remove-old-eventextras-api branch January 25, 2023 10:18
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