Skip to content

Conversation

@aliu39
Copy link
Member

@aliu39 aliu39 commented Jan 6, 2025

Follow-up to #3860, which hasn't been released yet

@codecov
Copy link

codecov bot commented Jan 6, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
13784 1 13783 4153
View the top 1 failed tests by shortest run time
tests.test_transport test_transport_works[threads-False-gzip-0-True-flush-False]
Stack Traces | 2.52s run time
tests/test_transport.py:187: in test_transport_works
    assert capturing_server.captured
E   AssertionError: assert []
E    +  where [] = <CapturingServer(<class 'tests.test_transport.CapturingServer'>, started 139966323615488)>.captured

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

This is a breaking change and needs a major update for the SDK.

Is this something we MUST have or it is just a nice too have?
I would not want to make this breaking change only for cosmetic reasons.

@antonpirker
Copy link
Contributor

@aliu39 and @cmanallen the FeatureFlagsIntegration is not documented yet so I guess it is still in beta or some kind of stealth mode, right?
If it is not yet used by a lot of users, we probably can rename it without a major update.

Copy link
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

In our feature flag documentation we state that feature flag tracking is a beta feature.

Also FeatureFlagsIntegration is not yet documented at all.

So renaming this is OK in a minor release.

@antonpirker antonpirker merged commit bf65ede into master Jan 7, 2025
137 of 138 checks passed
@cmanallen
Copy link
Member

@antonpirker The featureflags modules hasn't been released yet so unless someone is running from master no one has ever used the integration. That's why we felt it was safe. Thanks for merging this! :)

@antonpirker antonpirker deleted the aliu/rename-ff-module branch April 22, 2025 09:30
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.

4 participants