Skip to content

ref(flags): underscore featureflags module and identifier#3902

Merged
antonpirker merged 4 commits into
masterfrom
aliu/rename-ff-module
Jan 7, 2025
Merged

ref(flags): underscore featureflags module and identifier#3902
antonpirker merged 4 commits into
masterfrom
aliu/rename-ff-module

Conversation

@aliu39

@aliu39 aliu39 commented Jan 6, 2025

Copy link
Copy Markdown
Member

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

This was referenced Jan 6, 2025
Closed
Merged
@codecov

codecov Bot commented Jan 6, 2025 •
edited
Loading

Copy link
Copy Markdown

❌ 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!

antonpirker
antonpirker suggested changes Jan 7, 2025
View reviewed changes

@antonpirker antonpirker left a comment

Copy link
Copy Markdown
Contributor

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

antonpirker commented Jan 7, 2025

Copy link
Copy Markdown
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.

antonpirker
antonpirker approved these changes Jan 7, 2025
View reviewed changes

@antonpirker antonpirker left a comment

Copy link
Copy Markdown
Contributor

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
@cmanallen

cmanallen commented Jan 7, 2025

Copy link
Copy Markdown
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

Reviewers

@cmanallen cmanallen cmanallen approved these changes
+1 more reviewer
@antonpirker antonpirker antonpirker approved these changes
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

@aliu39 @antonpirker @cmanallen