-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: added fix for disabling the autocommit feature without feature flag #34015
Conversation
WalkthroughThe primary change involves adding a new feature flag annotation Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Failed server tests
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperImpl.java (1 hunks)
Additional comments not posted (5)
app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperImpl.java (5)
Line range hint
94-113
: The implementation of auto-commit eligibility checks is well-done, combining both server and client requirements effectively.
Line range hint
31-67
: The implementation of server auto-commit eligibility looks robust. However, ensure that the locking mechanism is tested thoroughly to avoid potential deadlocks.
Line range hint
31-31
: Ensure the feature flag is correctly set in the configuration.
94-94
: Ensure the feature flag is correctly set in the configuration.Verification successful
The feature flag
release_git_autocommit_feature_enabled
is indeed present in multiple locations within the codebase, including configuration files and test files. This confirms that the feature flag is correctly set and utilized.
app/client/src/ce/entities/FeatureFlag.ts
: The feature flag is defined here.app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperImpl.java
: The feature flag is used here.app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java
: The feature flag is enumerated here.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the feature flag is set in the configuration files. # Test: Search for the feature flag in the configuration files. Expect: At least one occurrence. rg --type yaml 'release_git_autocommit_feature_enabled'Length of output: 55
Script:
#!/bin/bash # Description: Verify the feature flag is set in the configuration files. # Test: Search for the feature flag in Java properties files. Expect: At least one occurrence. rg --type properties 'release_git_autocommit_feature_enabled' # Test: Search for the feature flag in environment variable files. Expect: At least one occurrence. rg --type env 'release_git_autocommit_feature_enabled'Length of output: 179
Script:
#!/bin/bash # Description: Verify the feature flag is set in the configuration files. # Test: Search for the feature flag in all files. Expect: At least one occurrence. rg 'release_git_autocommit_feature_enabled'Length of output: 3704
Line range hint
69-69
: Ensure the feature flag is correctly set in the configuration.
Description
(release_git_autocommit_enabled)
to the interface methodisAutoCommitRequired
forAutocommitEligibilityHelper
to disable the autocommit as a featureFixes #
Issue Number
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Git"
馃攳 Cypress test results
Tip
馃煝 馃煝 馃煝 All cypress tests have passed! 馃帀 馃帀 馃帀
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9397519853
Commit: 55da1e6
Cypress dashboard url: Click here!
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit