s0 #4964
Open
s0 #4964
+460,337
−30,092
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying windmill with
|
| Latest commit: |
402db01
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e4cff03c.windmill.pages.dev |
| Branch Preview URL: | https://rf-sandpack.windmill.pages.dev |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f8e31c2 in 1 minute and 30 seconds
More details
- Looked at
1922lines of code in28files - Skipped
1files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. frontend/src/lib/components/apps/editor/appUtils.ts:1285
- Draft comment:
Theanimatefunction is defined but never used. Consider removing it if it's not needed. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/lib/components/apps/editor/appUtils.ts:1281
- Draft comment:
TheanimateTofunction is defined in multiple files. Consider centralizing it in a utility file to avoid duplication. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment suggests there's duplication but doesn't provide proof. The function is being newly added here and we don't have visibility into other files to verify the claim. Without strong evidence of duplication, we should be skeptical of this comment. Additionally, even if the function exists elsewhere, we'd need to see those implementations to know if they're truly duplicate or if they have subtle differences for different use cases.
I could be wrong if this is a common animation utility that exists in other frontend files. The function name is generic enough that it might be duplicated.
While duplication is possible, we need clear evidence before asking for changes. The rules state we must see STRONG EVIDENCE that a comment is correct to keep it.
Delete this comment since we don't have strong evidence of duplication and can't verify the claim without seeing other files.
3. frontend/src/lib/editorUtils.ts:92
- Draft comment:
TheextToLangfunction is redundant withgetLangOfExtin+page.svelte. Consider consolidating these functions to avoid redundancy. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/lib/editorUtils.ts:47
- Draft comment:
EnsurelangToExtis consistent with other parts of the codebase to avoid redundancy. - Reason this comment was not posted:
Confidence changes required:50%
ThelangToExtfunction ineditorUtils.tsis comprehensive but might have overlapping logic with other parts of the codebase. It's important to ensure consistency across the codebase.
5. frontend/src/lib/scripts.ts:7
- Draft comment:
EnsurescriptLangToEditorLangis consistent with other parts of the codebase to avoid redundancy. - Reason this comment was not posted:
Confidence changes required:50%
ThescriptLangToEditorLangfunction inscripts.tsis comprehensive but might have overlapping logic with other parts of the codebase. It's important to ensure consistency across the codebase.
Workflow ID: wflow_iQkXr6pWDrJWmSN2
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| // code here | ||
| }) | ||
|
|
||
| function getLangOfExt(path: string) { |
There was a problem hiding this comment.
The getLangOfExt function is redundant. Consider using extToLang from editorUtils.ts instead.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3ec7bda in 25 seconds
More details
- Looked at
45lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_OkaWFCaQEycGC62p
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3e3271d in 30 seconds
More details
- Looked at
503lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. frontend/src/lib/components/RawAppEditor.svelte:95
- Draft comment:
Remove console.log statements to avoid unnecessary logging in production code. - Reason this comment was not posted:
Confidence changes required:50%
The code inRawAppEditor.sveltecontains a console.log statement that should be removed for production code.
2. frontend/src/lib/components/RawAppEditor.svelte:152
- Draft comment:
Consider checking if the file already exists before creating a new one to avoid overwriting or duplicating files. - Reason this comment was not posted:
Confidence changes required:50%
The code inRawAppEditor.sveltehas a potential issue with thecreateNewFilefunction where it doesn't check if the file already exists before creating a new one.
3. frontend/src/lib/components/RawAppEditor.svelte:105
- Draft comment:
.jsxfiles should return 'javascript' instead of 'typescript'. - Reason this comment was not posted:
Confidence changes required:50%
ThegetLangOfExtfunction inRawAppEditor.sveltehas a minor issue where it returns 'typescript' for '.jsx' files, which should be 'javascript'.
Workflow ID: wflow_bJwRHtNv1iRiLFL2
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on b02412e in 1 minute and 0 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. backend/windmill-worker/src/worker_lockfiles.rs:1622
- Draft comment:
Checking for file names with a leading slash (e.g., "/index.tsx") might not be necessary. Consider normalizing file names to avoid leading slashes. - Reason this comment was not posted:
Confidence changes required:50%
The code checks for file names with a leading slash, which might not be necessary. The file names should be consistent without leading slashes.
Workflow ID: wflow_OezlnONdK0mfCH6B
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f3d1c77 in 1 minute and 9 seconds
More details
- Looked at
52lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/routes/(root)/(logged)/apps_raw/add/templates.ts:603
- Draft comment:
The JSX syntax is incorrect. The closing tag for the App component should be self-closing or have a corresponding opening tag.
root.render(<App />);
- Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_7UhC9cL8SIvhdYe6
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ea0d0f7 in 2 minutes and 10 seconds
More details
- Looked at
1029lines of code in14files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. backend/windmill-worker/src/worker_lockfiles.rs:1628
- Draft comment:
TheAppFilestruct has been removed and replaced with aStringin theupload_raw_appfunction. Ensure that all code interacting withRawAppValueis updated to handle this change. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/lib/components/AppConnectInner.svelte:112
- Draft comment:
The try-catch block forOauthService.listOauthConnects()improves error handling. Ensure the error message is user-friendly and provides enough context for troubleshooting. - Reason this comment was not posted:
Confidence changes required:30%
The try-catch block added aroundOauthService.listOauthConnects()is a good addition for error handling. However, ensure that the error message is user-friendly and provides enough context for troubleshooting.
3. frontend/src/lib/components/Editor.svelte:749
- Draft comment:
The change fromconnectionProvidertomessageTransportsinMonacoLanguageClientinitialization should be verified for compatibility with the rest of the code. - Reason this comment was not posted:
Confidence changes required:50%
The change fromconnectionProvidertomessageTransportsin theMonacoLanguageClientinitialization is a significant change. Ensure that this change is compatible with the rest of the code and does not introduce any issues.
4. frontend/src/lib/components/raw_apps/RawAppEditorHeader.svelte:561
- Draft comment:
Ensure that the removal of theonKeyDownfunction does not affect any required keyboard shortcuts or functionalities. - Reason this comment was not posted:
Confidence changes required:40%
The removal of theonKeyDownfunction inRawAppEditorHeader.sveltemight affect keyboard shortcuts or other functionalities. Ensure that this functionality is not needed or is handled elsewhere.
Workflow ID: wflow_B62f4zg136SK5mPj
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 410e03f in 2 minutes and 58 seconds
More details
- Looked at
781lines of code in4files - Skipped
0files when reviewing. - Skipped posting
7drafted comments based on config settings.
1. frontend/src/lib/components/raw_apps/readonly_filesystem.ts:119
- Draft comment:
Remove console.log statements used for debugging purposes to clean up the code before production. This applies to other console.log statements in this file as well. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/lib/components/raw_apps/readonly_filesystem.ts:138
- Draft comment:
Remove console.log statements used for debugging purposes to clean up the code before production. This applies to other console.log statements in this file as well. - Reason this comment was not posted:
Marked as duplicate.
3. frontend/src/lib/components/raw_apps/readonly_filesystem.ts:159
- Draft comment:
Remove console.log statements used for debugging purposes to clean up the code before production. This applies to other console.log statements in this file as well. - Reason this comment was not posted:
Marked as duplicate.
4. frontend/src/lib/components/raw_apps/vscode.ts:51
- Draft comment:
Remove commented-out code sections to keep the codebase clean and maintainable. This applies to other similar sections in this file. - Reason this comment was not posted:
Confidence changes required:50%
The code contains commented-out sections that are not needed for the current implementation. These should be removed to keep the code clean and maintainable.
5. frontend/src/lib/components/raw_apps/vscode.ts:69
- Draft comment:
Remove commented-out code sections to keep the codebase clean and maintainable. This applies to other similar sections in this file. - Reason this comment was not posted:
Confidence changes required:50%
The code contains commented-out sections that are not needed for the current implementation. These should be removed to keep the code clean and maintainable.
6. frontend/src/lib/components/raw_apps/vscode.ts:110
- Draft comment:
Remove commented-out code sections to keep the codebase clean and maintainable. This applies to other similar sections in this file. - Reason this comment was not posted:
Confidence changes required:50%
The code contains commented-out sections that are not needed for the current implementation. These should be removed to keep the code clean and maintainable.
7. frontend/src/lib/components/raw_apps/vscode.ts:131
- Draft comment:
Remove commented-out code sections to keep the codebase clean and maintainable. This applies to other similar sections in this file. - Reason this comment was not posted:
Confidence changes required:50%
The code contains commented-out sections that are not needed for the current implementation. These should be removed to keep the code clean and maintainable.
Workflow ID: wflow_uDaAQBMZMSrFHa53
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| } | ||
|
|
||
| private _findNode(path: string): FileNode | undefined { | ||
| console.log('FIND NODE', path) |
There was a problem hiding this comment.
Remove console.log statements used for debugging purposes to clean up the code before production. This applies to other console.log statements in this file as well.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on a18edd3 in 1 minute and 38 seconds
More details
- Looked at
313lines of code in8files - Skipped
1files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/DiffEditor.svelte:5
- Draft comment:
The import of '@codingame/monaco-vscode-standalone-languages' is commented out. If it's not needed, consider removing it to keep the code clean. This applies to other files as well. - Reason this comment was not posted:
Confidence changes required:50%
The import of '@codingame/monaco-vscode-standalone-languages' is commented out in multiple files. This might be intentional, but if it's not needed, it should be removed to keep the code clean.
Workflow ID: wflow_Rfjlrdy4VJdM9GJF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
Skipped PR review on bf95ede because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Enhance raw app management with
tsxandjsonsupport, new templates, and editor updates.RawAppEditor.sveltefor handling raw apps.templates.ts.+page.svelteand+page.js.tsxandjsonsupport ineditorUtils.ts.RawAppEditor.svelteto handletsxandjsonfiles.vite.config.jsto excludemonaco-editor-wrapperfrom optimization.app-preview.htmlfor app previews.This description was created by
for a18edd3. It will automatically update as commits are pushed.