Adding windows support for studio:export#10075
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enables the Firebase Studio migration feature to run on Windows by removing platform-specific restrictions and implementing Windows-compatible logic for command execution and tool detection. The changes primarily focus on adapting the migration process to the Windows environment, ensuring a consistent user experience across platforms. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds Windows support for the studio:export command. The changes correctly remove the platform block, add logic to find the agy executable on Windows, and use the shell: true option when spawning the process. The tests have been updated accordingly. My review includes a few suggestions in the test files to adhere to the repository's style guide by avoiding the use of any.
| } as Response); | ||
|
|
||
| // Mock filesystem | ||
| sandbox.stub(fs, "readFile").callsFake(async (p: any) => { |
There was a problem hiding this comment.
The use of any for the parameter p violates the repository's style guide. Please use a more specific type. Since fs.readFile accepts a fs.PathLike argument, you can use that type here.
| sandbox.stub(fs, "readFile").callsFake(async (p: any) => { | |
| sandbox.stub(fs, "readFile").callsFake(async (p: fs.PathLike) => { |
References
- The repository style guide prohibits using
anyas an escape hatch. It's recommended to use proper types or interfaces. (link)
| sandbox.stub(fs, "mkdir").resolves(); | ||
| sandbox.stub(fs, "unlink").resolves(); | ||
| sandbox.stub(fs, "readdir").resolves([]); | ||
| sandbox.stub(fs, "access").callsFake(async (p: any) => { |
There was a problem hiding this comment.
Similar to the readFile stub, using any for the parameter p here violates the style guide. Please use a more specific type like fs.PathLike.
| sandbox.stub(fs, "access").callsFake(async (p: any) => { | |
| sandbox.stub(fs, "access").callsFake(async (p: fs.PathLike) => { |
References
- The repository style guide prohibits using
anyas an escape hatch. It's recommended to use proper types or interfaces. (link)
| unref: () => { | ||
| // No-op for testing | ||
| }, | ||
| } as any); |
There was a problem hiding this comment.
The type assertion as any violates the repository's style guide, which advises against using any as an escape hatch. To align with best practices, you can use as unknown as import("child_process").ChildProcess for better type safety.
| } as any); | |
| } as unknown as import("child_process").ChildProcess); |
References
- The repository style guide prohibits using
anyas an escape hatch. It's recommended to use proper types or interfaces. (link)
|
|
||
| expect(loginTool).to.exist; | ||
| }).timeout(2000); | ||
| }).timeout(4000); |
There was a problem hiding this comment.
Unrelated, but this was annoying me on CI
| exit: 1, | ||
| }); | ||
| } | ||
| // No-op for platform support check now |
There was a problem hiding this comment.
nit: can omit this comment
* Adding windows support * PR fixes * Format * Format * PR fix
Description
Windows support for studio:export. Tested on a personal Windows machine and confirmed that the export is successful + Antigravity starts up