Ask the user which backend they want to use. #10087
Ask the user which backend they want to use. #10087christhompsongoogle merged 7 commits intomainfrom
Conversation
…n that it quits if invalid but it can be re-ran.
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 enhances the App Hosting migration process by introducing an interactive step for backend selection. Instead of automatically assigning a backend, users are now prompted to confirm a suggested backend or choose from a list of available options, providing greater control and clarity during the setup. 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
Activity
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces an interactive flow for selecting an App Hosting backend, allowing users to confirm a suggested backend or choose from a list. This is a valuable improvement for user experience by providing more control to the user during the migration process. My feedback primarily focuses on improving the code's maintainability by reducing nesting in the newly added logic.
| let selectedBackendId = ""; | ||
| if (studioBackend) { | ||
| backendId = studioBackend.name.split("/").pop()!; | ||
| selectedBackendId = studioBackend.name.split("/").pop()!; | ||
| } else { | ||
| selectedBackendId = backendIds[0]; | ||
| } | ||
|
|
||
| const confirmBackend = await prompt.confirm({ | ||
| message: `Would you like to use the App Hosting backend "${selectedBackendId}"?`, | ||
| default: true, | ||
| }); | ||
|
|
||
| if (confirmBackend) { | ||
| backendId = selectedBackendId; | ||
| } else { | ||
| backendId = backends[0].name.split("/").pop()!; | ||
| logger.info("Available App Hosting backends:"); | ||
| for (const id of backendIds) { | ||
| logger.info(` - ${id}`); | ||
| } | ||
|
|
||
| const inputBackendId = await prompt.input({ | ||
| message: "Please enter the name of the backend you would like to use:", | ||
| }); | ||
|
|
||
| if (!backendIds.includes(inputBackendId)) { | ||
| throw new FirebaseError(`Invalid backend selected: ${inputBackendId}`, { exit: 1 }); | ||
| } | ||
| backendId = inputBackendId; | ||
| } |
There was a problem hiding this comment.
The interactive backend selection logic introduces significant nesting within the createFirebaseConfigs function. To improve readability and maintainability, and to adhere to the best practice of reducing nesting as much as possible (Repository Style Guide, line 30), please extract this logic into a new asynchronous helper function, for example, selectBackendInteractively. This function would encapsulate the prompts and validation, returning the chosen backendId.
| let selectedBackendId = ""; | |
| if (studioBackend) { | |
| backendId = studioBackend.name.split("/").pop()!; | |
| selectedBackendId = studioBackend.name.split("/").pop()!; | |
| } else { | |
| selectedBackendId = backendIds[0]; | |
| } | |
| const confirmBackend = await prompt.confirm({ | |
| message: `Would you like to use the App Hosting backend "${selectedBackendId}"?`, | |
| default: true, | |
| }); | |
| if (confirmBackend) { | |
| backendId = selectedBackendId; | |
| } else { | |
| backendId = backends[0].name.split("/").pop()!; | |
| logger.info("Available App Hosting backends:"); | |
| for (const id of backendIds) { | |
| logger.info(` - ${id}`); | |
| } | |
| const inputBackendId = await prompt.input({ | |
| message: "Please enter the name of the backend you would like to use:", | |
| }); | |
| if (!backendIds.includes(inputBackendId)) { | |
| throw new FirebaseError(`Invalid backend selected: ${inputBackendId}`, { exit: 1 }); | |
| } | |
| backendId = inputBackendId; | |
| } | |
| backendId = await selectBackendInteractively(backendIds, studioBackend); |
References
- Code should avoid unnecessarily deep nesting or long periods of nesting. Handle edge cases early and exit or fold them into the general case. Consider helper functions that can completely encapsulate branching, e.g. multiple ways a variable can be populated. (link)
There was a problem hiding this comment.
Good suggestion - also, do we want to use a select prompt here, or are we intentionally choosing a text input prompt to be more agent friendly?
There was a problem hiding this comment.
text input for flexibility mostly - this one is coming in late so I took the simple route rather than have a complicated selector
joehan
left a comment
joehan
left a comment
There was a problem hiding this comment.
Approving pending fixing the test
2 participants
This is a little crude in that it quits if invalid but it can be re-ran.