Skip to content

Conversation

@haacked
Copy link
Collaborator

@haacked haacked commented Feb 11, 2025

It's no longer used anywhere. This is just cleanup code.

It's no longer used anywhere.
@haacked haacked requested a review from a team February 11, 2025 02:49
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR removes the unused is_simple_flag attribute from feature flag definitions in test files, focusing on code cleanup without impacting functionality.

  • Removed is_simple_flag attribute from test data in /posthog/test/test_client.py
  • Removed is_simple_flag attribute from test data in /posthog/test/test_feature_flags.py
  • All tests continue to pass as the attribute was not used in any logical evaluations
  • Feature flag behavior remains unchanged, still evaluating based on core properties like active, filters, groups, and rollout_percentage

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@havenbarnes havenbarnes left a comment

Choose a reason for hiding this comment

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

This makes sense to me - thoughts on removing the column with a django migration?

@haacked
Copy link
Collaborator Author

haacked commented Feb 11, 2025

This makes sense to me - thoughts on removing the column with a django migration?

I'll look into that. I wanted to remove it from the client tests first.

@haacked haacked merged commit 3ff2a85 into master Feb 11, 2025
2 checks passed
@haacked haacked deleted the haacked/remove-is-simple-flag branch February 11, 2025 02:52
@haacked
Copy link
Collaborator Author

haacked commented Feb 11, 2025

This makes sense to me - thoughts on removing the column with a django migration?

The good news is it's not a column on the posthog_featureflag. AFAICT, it's always been a computed property.

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.

3 participants