Skip to content

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Sep 5, 2025

This pull request introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios (PR, nightly, prerelease) correctly. It also includes logic for both local runs (with a local quickstart repo) and CI runs (cloning the repo). My review includes a few minor suggestions to improve the robustness and maintainability of the script.

#no-changelog

Copy link
Contributor

Using Gemini Code Assist

The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

@ncooke3 ncooke3 requested a review from andrewheard September 5, 2025 21:24
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 5, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new script, scripts/setup_quickstart_spm.sh, to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, using set -euo pipefail for robustness and providing clear usage instructions. My review identifies a few areas for improvement. The most critical issue is a hardcoded git checkout to a specific branch, which should be removed to make the script generally usable. Other suggestions include minor shell scripting best practices, such as consistently using [[ ... ]] for tests and quoting variables and paths to enhance script reliability.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new script, setup_quickstart_spm.sh, to automate the setup of SPM-based quickstarts for integration testing. The script is well-structured, with clear argument parsing and different modes for various testing scenarios. My review focuses on improving the script's robustness and maintainability by addressing a hardcoded branch name, suggesting the use of more modern bash syntax, and ensuring variables are properly quoted.

Copy link
Contributor

@andrewheard andrewheard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Most of the Gemini suggestions also seem reasonable to me apart from changing mc/spm, which I think was intentional (but please double check my understanding).

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
ncooke3 and others added 4 commits September 5, 2025 18:04
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 5, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new script, setup_quickstart_spm.sh, to automate the setup of SPM-based quickstarts for integration testing. The script is well-structured and covers different testing scenarios. My review focuses on improving its robustness and maintainability. I've suggested changes to more reliably locate the Xcode project file, ensure file paths are correctly resolved to absolute paths, and enhance the readability of the logic for selecting testing modes.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 5, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios (PR, nightly, prerelease) correctly. It also includes logic for both local runs (with a local quickstart repo) and CI runs (cloning the repo). My review includes a few minor suggestions to improve the robustness and maintainability of the script.

ncooke3 and others added 3 commits September 5, 2025 19:55
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios (PR, nightly, prerelease) correctly. It also includes logic for both local runs (with a local quickstart repo) and CI runs (cloning the repo). My review includes a few suggestions to improve the robustness and maintainability of the script, including fixing a bug related to an unused environment variable and simplifying some logic.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script, scripts/setup_quickstart_spm.sh, to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and correctly handles different testing scenarios. The removal of set -x from scripts/check_secrets.sh is also a good change, allowing calling scripts to control execution tracing. My review includes one minor suggestion to improve the new script's readability.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script, scripts/setup_quickstart_spm.sh, to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and correctly handles different testing scenarios. It also includes robust logic for both local runs and CI runs. My review includes a couple of minor suggestions to improve the script's documentation and maintainability. The change in scripts/check_secrets.sh is a good cleanup.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios correctly. My review includes one high-priority suggestion to improve the robustness of SPM dependency pinning and a minor suggestion to improve the script's maintainability.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and correctly handles different testing scenarios. It also includes logic for both local and CI runs. My review includes one minor suggestion to improve the readability of a complex conditional check. The removal of set -x from check_secrets.sh is also a good improvement to reduce log noise.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios correctly. It also includes logic for both local runs and CI runs. The accompanying change to scripts/check_secrets.sh is a good cleanup. My review includes a few minor suggestions to improve the new script.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new, well-structured bash script to automate the setup of SPM-based quickstart projects. The script correctly handles various testing scenarios and environments. The change to check_secrets.sh is also a good cleanup. My review includes a couple of minor suggestions for the new script to enhance its robustness.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 6, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios correctly. It also includes logic for both local runs (with a local quickstart repo) and CI runs (cloning the repo). My review includes one minor suggestion to improve the robustness of the script for local development runs. The change in scripts/check_secrets.sh is a simple cleanup to remove debug output.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ncooke3
Copy link
Member Author

ncooke3 commented Sep 7, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script, scripts/setup_quickstart_spm.sh, to automate the setup of SPM-based quickstart projects. The script is well-written, following shell best practices and handling various testing scenarios robustly. The change to scripts/check_secrets.sh is also a good improvement. I have a couple of minor suggestions to further improve the new script's readability and maintainability.

@ncooke3
Copy link
Member Author

ncooke3 commented Sep 7, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a new bash script scripts/setup_quickstart_spm.sh to automate the setup of SPM-based quickstart projects for integration testing. The script is well-structured, uses shell best practices like set -euo pipefail, and handles different testing scenarios correctly. It also includes logic for both local runs (with a local quickstart repo) and CI runs (cloning the repo). The removal of set -x from scripts/check_secrets.sh is also a good improvement. My review includes one minor suggestion to improve the robustness of the new script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants