Skip to content

Report Flutter framework feature flags#2991

Merged
buenaflor merged 22 commits into
getsentry:mainfrom
ueman:add-framework-feature-flags
Jul 15, 2025
Merged

Report Flutter framework feature flags#2991
buenaflor merged 22 commits into
getsentry:mainfrom
ueman:add-framework-feature-flags

Conversation

@ueman

@ueman ueman commented Jun 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📜 Description

There's a Flutter framework PR (flutter/flutter#168437, flutter/flutter#171545 (reland of the first PR)) that adds feature flags to Flutter itself.
This PR supports that

💡 Motivation and Context

I want know whether a specific build used one of the feature flags.

💚 How did you test it?

Automated tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

I'll keep this a draft until the linked PR is merged for some time. Afterwards this PR can be landed.

@codecov

codecov Bot commented Jun 14, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.89%. Comparing base (81f83eb) to head (31bf545).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ns/flutter_framework_feature_flag_integration.dart 92.30% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2991      +/-   ##
==========================================
+ Coverage   87.74%   88.89%   +1.15%     
==========================================
  Files         286       97     -189     
  Lines        9719     3522    -6197     
==========================================
- Hits         8528     3131    -5397     
+ Misses       1191      391     -800

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ueman
ueman commented Jun 17, 2025
View reviewed changes
Comment thread flutter/test/integrations/flutter_framework_feature_flag_integration_test.dart
buenaflor
buenaflor reviewed Jun 17, 2025
View reviewed changes
Comment thread flutter/lib/src/integrations/flutter_framework_feature_flag_integration.dart Outdated
Comment thread flutter/test/integrations/flutter_framework_feature_flag_integration_test.dart
ueman
ueman commented Jun 17, 2025
View reviewed changes
Comment thread flutter/test/integrations/flutter_framework_feature_flag_integration_test.dart Outdated
ueman and others added 2 commits June 17, 2025 18:39
ueman
ueman commented Jul 2, 2025
View reviewed changes
Comment thread flutter/lib/src/integrations/flutter_framework_feature_flag_integration.dart Outdated
@ueman
ueman marked this pull request as ready for review July 3, 2025 05:18
buenaflor
buenaflor reviewed Jul 3, 2025
View reviewed changes
Comment thread flutter/lib/src/integrations/flutter_framework_feature_flag_integration.dart Outdated
@ueman

ueman commented Jul 3, 2025

Copy link
Copy Markdown
Collaborator Author

I would still wait with merging this PR until the linked PR is merged

loic-sharma
loic-sharma reviewed Jul 3, 2025
View reviewed changes
Comment thread flutter/lib/src/integrations/flutter_framework_feature_flag_integration.dart Outdated
// if it's not in there.
// As a result, we also don't know the whole list of flags, but only the active ones.
//
// See https://github.com/flutter/flutter/pull/168437

@loic-sharma loic-sharma Jul 3, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hello, I'm sure you're aware but this reaches into Flutter internals that aren't meant to be public. This isn't covered by Flutter's breaking change policy. I'd assume the format of FLUTTER_ENABLED_FEATURE_FLAGS can and will change, even in a patch release. The current changes seem reasonable, but I'd consider documenting this so that future contributors are aware as well.

ueman
ueman commented Jul 3, 2025
View reviewed changes
Comment thread flutter/lib/src/integrations/flutter_framework_feature_flag_integration.dart Outdated
@ueman

ueman commented Jul 15, 2025

Copy link
Copy Markdown
Collaborator Author

@buenaflor The dependent PR in the Flutter framework has landed, so I would say we can go ahead and merge this PR

cursor[bot]

This comment was marked as outdated.

Sign in to view

@buenaflor

buenaflor commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

@ueman sounds good

also does the cursor review make sense? if not we can just ignore it

@ueman

ueman commented Jul 15, 2025

Copy link
Copy Markdown
Collaborator Author

I assume those cases should not happen since the flags are supplied by the flutter tool and not user generated, but I added code and tests for them anyway

cursor[bot]

This comment was marked as outdated.

Sign in to view

@buenaflor
buenaflor merged commit 224aa18 into getsentry:main Jul 15, 2025
59 of 62 checks passed
@ueman
ueman deleted the add-framework-feature-flags branch July 15, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@buenaflor buenaflor buenaflor approved these changes
@stefanosiano stefanosiano Awaiting requested review from stefanosiano
@denrase denrase Awaiting requested review from denrase denrase is a code owner
+1 more reviewer
@loic-sharma loic-sharma loic-sharma left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ueman @buenaflor @loic-sharma