feat!: v2 #8
feat!: v2 #8
Conversation
# Conflicts: # package-lock.json # package.json # src/commands/build/index.ts # src/commands/dev/index.ts # src/commands/format/index.ts # src/commands/lint/index.ts # src/commands/preview/index.ts # src/commands/release/index.ts # src/context/PresetWorkflowProvider.ts # src/context/StageRunner.ts # src/context/context.ts # src/flow/WorkflowPresetRepository.ts # src/flow/WorkflowPresetRunner.ts # src/flow/WorkflowStage.ts # src/lib/workflow/WorkflowStages.ts # src/presets/default/HandleConfig.ts # src/presets/presets.ts # src/presets/release/index.ts # src/presets/rome/RomeBase.ts # src/presets/ts/ts.ts # src/task/FSTasks.ts # src/task/ProgramRunner.ts
WalkthroughThe project has been updated with new commands and enhancements, particularly focusing on the CLI tool's usability. A Changes
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 X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files ignored due to filter (5)
- package-lock.json
- package.json
- rome.json
- templates/rome/rome.json
- templates/vite/tsconfig.json
Files selected for processing (49)
- README.md (3 hunks)
- bin/dev.js (1 hunks)
- src/command/BaseCommand.ts (2 hunks)
- src/commands/build/index.ts (1 hunks)
- src/commands/dev/index.ts (1 hunks)
- src/commands/format/index.ts (1 hunks)
- src/commands/generate/index.ts (1 hunks)
- src/commands/lint/index.ts (1 hunks)
- src/commands/preview/index.ts (1 hunks)
- src/commands/release/index.ts (1 hunks)
- src/context/ConfigProvider.ts (2 hunks)
- src/context/ContextProvider.ts (2 hunks)
- src/context/PresetWorkflowProvider.ts (1 hunks)
- src/context/StageRunner.ts (2 hunks)
- src/context/context.ts (2 hunks)
- src/flow/WorkflowPresetRepository.ts (1 hunks)
- src/flow/WorkflowPresetRunner.ts (4 hunks)
- src/flow/WorkflowStage.ts (1 hunks)
- src/flow/WorkflowStages.ts (1 hunks)
- src/kplugin/KPlugin.ts (1 hunks)
- src/lib/flow/Action.ts (1 hunks)
- src/lib/flow/ActionGroup.ts (1 hunks)
- src/lib/fs/Files.ts (1 hunks)
- src/lib/fs/Location.ts (3 hunks)
- src/lib/fs/finder/Finder.ts (1 hunks)
- src/lib/fs/finder/IgnoreFinderFilter.ts (1 hunks)
- src/lib/plugin/Plugin.ts (1 hunks)
- src/lib/workflow/WorkflowStages.ts (1 hunks)
- src/lib/workflow/stage/StageAction.ts (1 hunks)
- src/lib/workflow/stage/StagePreset.ts (1 hunks)
- src/plugins/git/GitPlugin.ts (1 hunks)
- src/plugins/git/HandleConfig.ts (1 hunks)
- src/presets/default/HandleConfig.ts (1 hunks)
- src/presets/default/defaultPreset.ts (1 hunks)
- src/presets/presets.ts (1 hunks)
- src/presets/release/Release.ts (1 hunks)
- src/presets/release/ReleaseInit.ts (1 hunks)
- src/presets/release/index.ts (1 hunks)
- src/presets/rome/RomeBase.ts (2 hunks)
- src/presets/rome/RomeInit.ts (1 hunks)
- src/presets/rome/RomeLint.ts (1 hunks)
- src/presets/rome/index.ts (1 hunks)
- src/presets/ts/ViteBase.ts (2 hunks)
- src/presets/ts/ViteInit.ts (1 hunks)
- src/presets/ts/ts.ts (1 hunks)
- src/task/FSTasks.ts (2 hunks)
- src/task/ProgramRunner.ts (3 hunks)
- templates/vite/vite.ts.js (1 hunks)
- templates/vite/vite.ts.lib.js (1 hunks)
Files skipped from review due to trivial changes (34)
- README.md
- bin/dev.js
- src/command/BaseCommand.ts
- src/commands/dev/index.ts
- src/commands/generate/index.ts
- src/commands/lint/index.ts
- src/commands/preview/index.ts
- src/commands/release/index.ts
- src/context/ContextProvider.ts
- src/context/PresetWorkflowProvider.ts
- src/context/context.ts
- src/flow/WorkflowStage.ts
- src/flow/WorkflowStages.ts
- src/lib/flow/Action.ts
- src/lib/fs/Files.ts
- src/lib/fs/Location.ts
- src/lib/fs/finder/IgnoreFinderFilter.ts
- src/lib/workflow/WorkflowStages.ts
- src/lib/workflow/stage/StageAction.ts
- src/lib/workflow/stage/StagePreset.ts
- src/plugins/git/GitPlugin.ts
- src/presets/default/defaultPreset.ts
- src/presets/presets.ts
- src/presets/release/Release.ts
- src/presets/release/index.ts
- src/presets/rome/RomeBase.ts
- src/presets/rome/RomeInit.ts
- src/presets/rome/RomeLint.ts
- src/presets/rome/index.ts
- src/presets/ts/ViteBase.ts
- src/presets/ts/ViteInit.ts
- src/presets/ts/ts.ts
- templates/vite/vite.ts.js
- templates/vite/vite.ts.lib.js
Review comments generated (8)
- Actionable : 8
- LGTM (Skipped): 5
- Noise Suppression (Skipped): 20
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| async getConfigAsync(): Promise<KtoolsConfig> { | ||
| const gitIgnoreFilePath = this.context.project.ignoreFile.relativePath; | ||
|
|
||
| const fs = new Files(); | ||
| const gitIgnoreFile = await fs.readIfExists(".gitignore"); | ||
|
|
||
| const filters = []; | ||
|
|
||
| if (gitIgnoreFile) { | ||
| filters.push(new IgnoreFinderFilter(gitIgnoreFile)); | ||
| } | ||
|
|
||
| const finder = new Finder({ | ||
| filters, | ||
| }); | ||
|
|
||
| const files = await finder.findMatching(["**/*.*?js", "**/*.ts"]); | ||
|
|
||
| // console.log(this.context.project.ignoreFile.relativePath); | ||
| // console.log(files); | ||
| console.log(files.map(getFirstDirectory)); | ||
|
|
||
| return this.getConfig(); | ||
| } |
There was a problem hiding this comment.
This new getConfigAsync method reads a .gitignore file, finds all JavaScript and TypeScript files that are not ignored, and logs the first directory of each file. However, it doesn't seem to use this information for any purpose other than logging. It might be better to return this information or use it in some way. Also, the method ends by returning the result of this.getConfig(), which is a synchronous method. This could potentially block the event loop if the config loading operation is heavy. Consider making getConfig() asynchronous as well, or at least ensure that it won't block the event loop.
- return this.getConfig();
+ return await this.getConfigAsync();| import { WorkflowStage } from "../../flow/WorkflowStage.js"; | ||
| import { TaskList } from "../../lib/task/Task.js"; | ||
| import { Context } from "../../context/context.js"; | ||
|
|
||
| export class HandleConfig extends WorkflowStage { | ||
| run(): TaskList { | ||
| const { context } = Context; | ||
|
|
||
| return [ | ||
| Context.fs.copyConfigWhenMissing( | ||
| context.tools.config.filePath.absolutePath, | ||
| context.tools.config.fileName, | ||
| ), | ||
| ]; | ||
| } | ||
| } |
There was a problem hiding this comment.
The HandleConfig class extends WorkflowStage and overrides the run() method. It uses the Context.fs.copyConfigWhenMissing function to copy a configuration file if it's missing. However, there is no error handling in case the copying fails due to reasons such as lack of permissions or disk space. Consider adding error handling to this operation.
return [
Context.fs.copyConfigWhenMissing(
context.tools.config.filePath.absolutePath,
context.tools.config.fileName,
).catch(error => {
console.error(`Failed to copy config file: ${error}`);
throw error;
}),
];| constructor({ cwd, filters }: FinderOptions = defaultOptions) { | ||
| this.cwd = cwd; | ||
| this.filters = filters || []; | ||
| } |
There was a problem hiding this comment.
The constructor is using default parameters correctly. However, if the filters parameter is not provided, it will be set to an empty array. This could potentially overwrite any existing filters on the instance. If this is not the intended behavior, consider changing the logic here.
- this.filters = filters || [];
+ this.filters = filters ?? this.filters;| const configFile = Context.context.tools.config.fileName; | ||
| return [Context.fs.copyConfigWhenMissing(configFile, configFile)]; |
There was a problem hiding this comment.
The logic for copying the configuration file when it's missing has been simplified. However, the check for the project path existence has been removed. If this check is still necessary, consider adding it back.
- return [Context.fs.copyConfigWhenMissing(configFile, configFile)];
+ return [
+ Context.fs.projectPathExists(configFile),
+ Context.fs.copyConfigWhenMissing(configFile, configFile)
+ ];| projectPathExists(path: string): Task<{ projectConfigExists: boolean }> { | ||
| const file = this.context.project.rootPath.join(path).relativePath; | ||
|
|
||
| return { | ||
| title: `[FS] File exist ${file}`, | ||
| task: async (ctx, task) => { | ||
| ctx.projectConfigExists = !!(await jetpack.existsAsync(file)); | ||
| ctx.projectConfigExists = !!(await this.files.exists(file)); |
There was a problem hiding this comment.
The projectPathExists method has been refactored to use the Files class for file operations instead of directly using the fs-jetpack library. This change improves modularity and makes the code easier to test, as the Files class can be mocked during testing.
- ctx.projectConfigExists = !!(await jetpack.existsAsync(file));
+ ctx.projectConfigExists = !!(await this.files.exists(file));| await this.files.copyFile(localFile, projectFile); | ||
| task.title = `[FS] New config created ${projectFile}`; |
There was a problem hiding this comment.
The copyFile method from the Files class is used here to copy a file. This is an improvement over the previous implementation which used the fs-jetpack library directly. The task title is updated after the operation, providing more accurate feedback.
- await jetpack.copyAsync(localFile, projectFile);
+ await this.files.copyFile(localFile, projectFile);| await this.files.moveFile(originalFile, destinationFile); | ||
| task.title = `[FS] Config moved ${destinationFile}`; |
There was a problem hiding this comment.
The moveFile method from the Files class is used here to move a file. This is an improvement over the previous implementation which used the fs-jetpack library directly. The task title is updated after the operation, providing more accurate feedback.
- await jetpack.moveAsync(originalFile, destinationFile);
+ await this.files.moveFile(originalFile, destinationFile);| import { execa } from "execa"; | ||
| import { NpmProvider } from "../context/NpmProvider.js"; | ||
| import { TaskRunner } from "../lib/task/TaskRunner.js"; | ||
| import path from "node:path"; | ||
| import { Task, TaskContext } from "../lib/task/Task.js"; |
There was a problem hiding this comment.
The import statements are not ordered correctly. It's a good practice to order the imports in a certain way for better readability and maintainability. Usually, we start with third-party libraries, then internal modules. Also, it's common to sort them alphabetically.
- import { execa } from "execa";
- import { NpmProvider } from "../context/NpmProvider.js";
- import { TaskRunner } from "../lib/task/TaskRunner.js";
- import path from "node:path";
- import { Task, TaskContext } from "../lib/task/Task.js";
+ import { execa } from "execa";
+ import path from "node:path";
+ import { NpmProvider } from "../context/NpmProvider.js";
+ import { Task, TaskContext } from "../lib/task/Task.js";
+ import { TaskRunner } from "../lib/task/TaskRunner.js";
Watermelon AI SummaryThis pull request focuses on overhauling components for the second major version (v2) of the application. It includes a mix of updates such as introducing new features like a generic plugin system and a GitHub PRs
ktools is an open repo and Watermelon will serve it for free. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- bin/dev.js (2 hunks)
Additional comments: 2
bin/dev.js (2)
11-11: The updated comment clarifies the purpose of the code block. Ensure that the comment accurately reflects the code's functionality and intent.
21-21: The updated comment adds personality to the code. While this is not an issue, ensure that such comments maintain professionalism and clarity in the codebase.
Summary by CodeRabbit
New Features:
GenericPluginclass.GitPluginclass for handling Git-related tasks.Refactor:
WorkflowStageNameenum.Filesmodule functions.