-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Suggest how to run the dev app for the init prompt #9218
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @samedson, 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 enhances the developer experience within the Firestore initialization guide by providing clearer and more actionable instructions for verifying database setup. It specifically advises developers to run their local development application in a separate terminal and offers a practical tip for identifying the correct run command, particularly for web projects, thereby improving the overall testing workflow. Highlights
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the Firestore initialization guide to suggest how to run the development application for testing. My review includes two points:
- A high-severity concern about a potential contradiction between this guide and the
init_backend.ts
guide regarding when the application should be run for testing. - A medium-severity suggestion to make the instructions more comprehensive by including examples for running the app on different platforms, not just web.
- Only proceed with verification after successfully running \`firebase deploy --only firestore\` | ||
- Guide the user to visit \`https://console.firebase.google.com/u/0/project/{PROJECT_ID}/firestore\` where \`{PROJECT_ID}\` is the project they're currently using (or use \`_\` if project id is unknown). | ||
- Have developers test their application functionality and verify test data appears in the console | ||
- Have developers test their application functionality and verify test data appears in the console. Ask them to run a local version of their app in a separate terminal and suggest the command to run their dev app. For example, for web apps you can check their \`package.json\` to see how to run their app. |
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.
There appears to be a contradiction with the instructions in src/mcp/resources/guides/init_backend.ts
. Line 22 of that file states, "Do not run the app until you have completed all 3 guides" (which includes Firestore). However, this change adds instructions to run the app after setting up only Firestore. This conflicting guidance could confuse the agent. Please clarify the intended workflow and ensure the guides are consistent.
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.
WDYT about just removing that line about not running until all guides are done? Seems like that line would be a problem as we add other services anyway
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.
Gemini should run the app for them and provide the link to open the app on the browswer.
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.
Addressed both of these
- Only proceed with verification after successfully running \`firebase deploy --only firestore\` | ||
- Guide the user to visit \`https://console.firebase.google.com/u/0/project/{PROJECT_ID}/firestore\` where \`{PROJECT_ID}\` is the project they're currently using (or use \`_\` if project id is unknown). | ||
- Have developers test their application functionality and verify test data appears in the console | ||
- Have developers test their application functionality and verify test data appears in the console. Ask them to run a local version of their app in a separate terminal and suggest the command to run their dev app. For example, for web apps you can check their \`package.json\` to see how to run their app. |
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.
The example for running the dev app is specific to web applications. To make this instruction more comprehensive for the agent, which supports multiple platforms, it would be beneficial to include examples for other environments like Android and Flutter.
- Have developers test their application functionality and verify test data appears in the console. Ask them to run a local version of their app in a separate terminal and suggest the command to run their dev app. For example, for web apps check their `package.json` for a "start" or "dev" script; for Android apps, they can run from Android Studio; and for Flutter apps, they can use `flutter run`.
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.
Seems like a good clarification to me, wdyt?
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.
Added this
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.
LGTM in general, tho GCA's comments seem to be worth addressing
- Only proceed with verification after successfully running \`firebase deploy --only firestore\` | ||
- Guide the user to visit \`https://console.firebase.google.com/u/0/project/{PROJECT_ID}/firestore\` where \`{PROJECT_ID}\` is the project they're currently using (or use \`_\` if project id is unknown). | ||
- Have developers test their application functionality and verify test data appears in the console | ||
- Have developers test their application functionality and verify test data appears in the console. Ask them to run a local version of their app in a separate terminal and suggest the command to run their dev app. For example, for web apps you can check their \`package.json\` to see how to run their app. |
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.
Gemini should run the app for them and provide the link to open the app on the browswer.
No description provided.