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 1932322 - Implement the new collection-enabled API #3006

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

badboy
Copy link
Member

@badboy badboy commented Nov 21, 2024

This is the mega PR changing the inner workings of Glean to support collection-enabled and follows-collection-enabled on pings, to independently control a subset of pings and whether they can get submitted.

BREAKING CHANGE: This is definitely a huge breaking change.

2024-11-21: This is not yet fully ready for review

@badboy badboy force-pushed the collection-enabled branch 2 times, most recently from 92c1809 to bcff07a Compare November 21, 2024 13:07
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.

Lot's of little cleanup things, nothing major. I'd say this fits really well with my mental model of what this would look like from our discussions.

glean-core/android/build.gradle Outdated Show resolved Hide resolved
glean-core/ios/sdk_generator.sh Outdated Show resolved Hide resolved
glean-core/python/glean/glean.py Outdated Show resolved Hide resolved
glean-core/python/tests/test_collection_enabled.py Outdated Show resolved Hide resolved
glean-core/src/core/mod.rs Outdated Show resolved Hide resolved
glean-core/src/ping/mod.rs Outdated Show resolved Hide resolved
glean-core/tests/collection_enabled.rs Outdated Show resolved Hide resolved
glean-core/tests/event.rs Outdated Show resolved Hide resolved
samples/android/app/build.gradle Outdated Show resolved Hide resolved
@travis79
Copy link
Member

Also, docs/changelog/etc. ofc

@badboy badboy force-pushed the collection-enabled branch 6 times, most recently from a85c48b to e2a05ff Compare November 25, 2024 12:48
@badboy badboy marked this pull request as ready for review November 25, 2024 13:10
@badboy badboy requested a review from a team as a code owner November 25, 2024 13:10
@badboy badboy requested review from rosahbruno and removed request for a team November 25, 2024 13:10
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.54%. Comparing base (0a86c80) to head (e2a05ff).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3006   +/-   ##
=======================================
  Coverage   29.54%   29.54%           
=======================================
  Files           1        1           
  Lines          44       44           
=======================================
  Hits           13       13           
  Misses         31       31           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@badboy badboy removed the request for review from rosahbruno November 25, 2024 14:43
let pings_dir = self.get_pings_dir(data_path, None)?;

std::fs::remove_dir_all(&pings_dir)?;
create_dir_all(&pings_dir)?;
// TODO(bug 1932909): Refactor this into its own function
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@travis79
Copy link
Member

Looks good to me, thanks for filing bugs for the TODOs!

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