feat(cli): add flag ai removal command #151
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update enhances feature flag management within the application by introducing a new command for removing feature flags. Users can efficiently clean up unused flags, improving codebase maintenance. The addition includes a utility for generating regex patterns and a new dependency, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AI
participant HttpService
User->>CLI: Executes remove command
CLI->>AI: removeFlagInstance({flagName, options})
AI->>AI: Locate files using globby
AI->>AI: Construct regex for flag
AI->>HttpService: getFilesWithFlagsRemoved(data)
HttpService-->>AI: Response
AI-->>CLI: Result
CLI-->>User: Display result
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 Configuration File (
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (4)
packages/core/src/shared/helpers.ts (1)
71-72: Consider adding a comment to explain the purpose of the function.Adding a comment will help other developers understand the purpose and usage of this function.
+ // Generates a regex pattern to match the usage of a feature flag. export const getUseFeatureFlagRegex = (flagName: string) => new RegExp(`useFeatureFlag\\s*\\(\\s*['"\`]${flagName}['"\`]\\s*\\)`);packages/cli/src/http.ts (1)
74-84: Consider adding type annotations for the parameters.Adding type annotations will improve code readability and maintainability.
static async getFilesWithFlagsRemoved({ apiKey, files, flagName, apiUrl, }: { apiKey: string; files: Array<{ filePath: string; fileContent: string }>; flagName: string; apiUrl?: string; }) {packages/cli/src/index.ts (2)
193-194: Consider adding a description for the arguments.Adding a description will help users understand the purpose of the arguments.
.argument("<dir>", "The directory to scan for flags") .argument("<flag>", "The flag name to remove")
205-205: Log the result in a more user-friendly format.Logging the result in a more user-friendly format will improve the user experience.
console.log("Files updated:", files);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (6)
- .changeset/ninety-countries-jam.md (1 hunks)
- packages/cli/package.json (1 hunks)
- packages/cli/src/ai.ts (1 hunks)
- packages/cli/src/http.ts (2 hunks)
- packages/cli/src/index.ts (2 hunks)
- packages/core/src/shared/helpers.ts (1 hunks)
Additional comments not posted (7)
.changeset/ninety-countries-jam.md (1)
1-6: LGTM!The changeset metadata is correctly formatted and indicates patch updates for
@tryabby/coreand@tryabby/cli.packages/cli/package.json (1)
30-30: LGTM!The addition of
globbywith version^14.0.2is appropriate for enhancing file handling capabilities.packages/cli/src/ai.ts (5)
1-6: LGTM!The import statements are correctly structured and bring in the necessary modules for the
removeFlagInstancefunction.
15-20: LGTM!The usage of
globbyto find files with the specified options is appropriate and correctly implemented.
22-22: LGTM!The regex pattern generation using
getUseFeatureFlagRegexis correctly implemented.
41-46: LGTM!The HTTP service call to
getFilesWithFlagsRemovedis correctly implemented.
57-57: LGTM!The success message logging is correctly implemented.
packages/cli/src/http.ts
Outdated
| throw new Error("Internal server error trying to update files"); | ||
| } else if (status === 401) { | ||
| throw new Error("Invalid API Key"); | ||
| } else { | ||
| console.log(response); | ||
| throw new Error("Push failed"); | ||
| } |
There was a problem hiding this comment.
Improve error handling by providing more specific error messages.
Providing more specific error messages will help in debugging and understanding the cause of the error.
} else if (status === 500) {
throw new Error("Internal server error trying to update files");
} else if (status === 401) {
throw new Error("Invalid API Key");
} else {
console.log(response);
throw new Error(`Push failed with status ${status}: ${response.statusText}`);
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new Error("Internal server error trying to update files"); | |
| } else if (status === 401) { | |
| throw new Error("Invalid API Key"); | |
| } else { | |
| console.log(response); | |
| throw new Error("Push failed"); | |
| } | |
| } else if (status === 500) { | |
| throw new Error("Internal server error trying to update files"); | |
| } else if (status === 401) { | |
| throw new Error("Invalid API Key"); | |
| } else { | |
| console.log(response); | |
| throw new Error(`Push failed with status ${status}: ${response.statusText}`); | |
| } |
| try { | ||
| const { mutableConfig, saveMutableConfig } = await loadLocalConfig( | ||
| options.configPath | ||
| ); | ||
| mutableConfig.flags = mutableConfig.flags.filter((flag: string) => flag !== options.flagName); | ||
| await saveMutableConfig(); | ||
| } catch (e) { | ||
| // fail silently | ||
| } |
There was a problem hiding this comment.
Handle specific exceptions for configuration update.
The local configuration update is correctly implemented, but consider handling specific exceptions to provide more informative error messages.
- } catch (e) {
- // fail silently
+ } catch (error) {
+ console.error(`Error updating configuration:`, error);
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const { mutableConfig, saveMutableConfig } = await loadLocalConfig( | |
| options.configPath | |
| ); | |
| mutableConfig.flags = mutableConfig.flags.filter((flag: string) => flag !== options.flagName); | |
| await saveMutableConfig(); | |
| } catch (e) { | |
| // fail silently | |
| } | |
| try { | |
| const { mutableConfig, saveMutableConfig } = await loadLocalConfig( | |
| options.configPath | |
| ); | |
| mutableConfig.flags = mutableConfig.flags.filter((flag: string) => flag !== options.flagName); | |
| await saveMutableConfig(); | |
| } catch (error) { | |
| console.error(`Error updating configuration:`, error); | |
| } |
| const filesToUse = ( | ||
| await Promise.all( | ||
| files.flatMap(async (filePath) => { | ||
| const content = await readFile(filePath, "utf-8").then((content) => { | ||
| const matches = content.match(regex); | ||
| return matches ? content : null; | ||
| }); | ||
| if (!content) return []; | ||
|
|
||
| return { | ||
| filePath, | ||
| fileContent: content, | ||
| }; | ||
| }) | ||
| ) | ||
| ).flat(); |
There was a problem hiding this comment.
Ensure proper error handling for file reading.
The file reading and regex matching are correctly implemented, but consider adding error handling for potential file read errors.
- files.flatMap(async (filePath) => {
+ files.flatMap(async (filePath) => {
+ try {
const content = await readFile(filePath, "utf-8").then((content) => {
const matches = content.match(regex);
return matches ? content : null;
});
if (!content) return [];
return {
filePath,
fileContent: content,
};
+ } catch (error) {
+ console.error(`Error reading file ${filePath}:`, error);
+ return [];
+ }
})Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filesToUse = ( | |
| await Promise.all( | |
| files.flatMap(async (filePath) => { | |
| const content = await readFile(filePath, "utf-8").then((content) => { | |
| const matches = content.match(regex); | |
| return matches ? content : null; | |
| }); | |
| if (!content) return []; | |
| return { | |
| filePath, | |
| fileContent: content, | |
| }; | |
| }) | |
| ) | |
| ).flat(); | |
| const filesToUse = ( | |
| await Promise.all( | |
| files.flatMap(async (filePath) => { | |
| try { | |
| const content = await readFile(filePath, "utf-8").then((content) => { | |
| const matches = content.match(regex); | |
| return matches ? content : null; | |
| }); | |
| if (!content) return []; | |
| return { | |
| filePath, | |
| fileContent: content, | |
| }; | |
| } catch (error) { | |
| console.error(`Error reading file ${filePath}:`, error); | |
| return []; | |
| } | |
| }) | |
| ) | |
| ).flat(); |
cstrnt
force-pushed
the
tim07-29-feat_cli_add_flag_ai_removal_command
branch
from
5e950f9 to
d05cb9a
Compare
cstrnt
force-pushed
the
tim07-29-feat_cli_add_flag_ai_removal_command
branch
from
d05cb9a to
974324c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (24)
- apps/angular-example/CHANGELOG.md (1 hunks)
- apps/angular-example/package.json (1 hunks)
- apps/web/CHANGELOG.md (1 hunks)
- apps/web/package.json (1 hunks)
- packages/angular/CHANGELOG.md (1 hunks)
- packages/angular/package.json (1 hunks)
- packages/cli/CHANGELOG.md (1 hunks)
- packages/cli/package.json (2 hunks)
- packages/cli/src/ai.ts (1 hunks)
- packages/cli/src/http.ts (2 hunks)
- packages/cli/src/index.ts (2 hunks)
- packages/core/CHANGELOG.md (1 hunks)
- packages/core/package.json (1 hunks)
- packages/core/src/shared/helpers.ts (1 hunks)
- packages/next/CHANGELOG.md (1 hunks)
- packages/next/package.json (1 hunks)
- packages/node/CHANGELOG.md (1 hunks)
- packages/node/package.json (1 hunks)
- packages/react/CHANGELOG.md (1 hunks)
- packages/react/package.json (1 hunks)
- packages/remix/CHANGELOG.md (1 hunks)
- packages/remix/package.json (1 hunks)
- packages/svelte/CHANGELOG.md (1 hunks)
- packages/svelte/package.json (1 hunks)
Files skipped from review due to trivial changes (17)
- apps/angular-example/CHANGELOG.md
- apps/angular-example/package.json
- apps/web/CHANGELOG.md
- apps/web/package.json
- packages/angular/CHANGELOG.md
- packages/angular/package.json
- packages/core/package.json
- packages/next/CHANGELOG.md
- packages/next/package.json
- packages/node/CHANGELOG.md
- packages/node/package.json
- packages/react/CHANGELOG.md
- packages/react/package.json
- packages/remix/CHANGELOG.md
- packages/remix/package.json
- packages/svelte/CHANGELOG.md
- packages/svelte/package.json
Files skipped from review as they are similar to previous changes (5)
- packages/cli/package.json
- packages/cli/src/ai.ts
- packages/cli/src/http.ts
- packages/cli/src/index.ts
- packages/core/src/shared/helpers.ts
Additional comments not posted (2)
packages/cli/CHANGELOG.md (1)
3-8: Changelog entry looks good.The new version
1.1.1and the patch change for adding the feature flag removal command are correctly documented.packages/core/CHANGELOG.md (1)
3-8: Changelog entry looks good.The new version
5.3.1and the patch change for adding the feature flag removal command are correctly documented.
Summary by CodeRabbit
New Features
Improvements
globbylibrary for improved file path matching.Bug Fixes