Skip to content

fix: Autocommit feature flag in FeatureFlagEnum #36561

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.

Sign up for GitHub

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

Merged
nidhi-nair merged 1 commit into from
Sep 26, 2024
Merged

fix: Autocommit feature flag in FeatureFlagEnum #36561

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,9 @@ public enum FeatureFlagEnum {
release_embed_hide_share_settings_enabled,
rollout_datasource_test_rate_limit_enabled,

// Deprecated CE flags over here
release_git_autocommit_feature_enabled,
release_git_autocommit_eligibility_enabled,
Comment on lines +18 to +20
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Let's review the usage of the newly added deprecated feature flags.

Upon investigating, it appears that release_git_autocommit_feature_enabled is referenced solely in JsonSchemaVersionsFallback.java, and release_git_autocommit_eligibility_enabled isn't used elsewhere in the codebase. Introducing deprecated flags that aren't actively utilized can lead to unnecessary clutter and potential confusion for future maintenance.

I recommend evaluating whether these deprecated flags are essential for backward compatibility or if they can be safely removed to keep the codebase clean and maintainable.

🔗 Analysis chain

Class, let's discuss the addition of deprecated flags.

Now, students, I want you to pay close attention to lines 19 and 20. We've added two new feature flags here, but they're under a comment that says "Deprecated CE flags". Can anyone tell me why we might be adding deprecated flags?

This is an interesting teaching moment. Adding deprecated flags can sometimes be necessary for backward compatibility or for phasing out features gradually. However, we need to be cautious about this approach.

Let's do a little homework to understand how these flags are being used:

Remember, class, it's important to document why these flags are deprecated and when they should be removed. Let's make sure we're not cluttering our codebase with flags that aren't actively used or needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usage of the new feature flags

echo "Searching for usage of release_git_autocommit_feature_enabled:"
rg --type java "release_git_autocommit_feature_enabled"

echo "Searching for usage of release_git_autocommit_eligibility_enabled:"
rg --type java "release_git_autocommit_eligibility_enabled"

Length of output: 838


// Add EE flags below this line, to avoid conflicts.
}