-
Notifications
You must be signed in to change notification settings - Fork 2k
ci(ocr): migrate to new CI #4009
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
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.
Hello @glasnt, 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!
Summary of Changes
This pull request migrates the OCR function to a new CI setup. The changes involve adding the functions/ocr/app to the nodejs-dev.jsonc configuration file, removing it from the nodejs-prod.jsonc file, and adding a ci-setup.json file to the functions/ocr/app directory to define environment variables for the CI environment.
Highlights
- CI Migration: The primary goal is to migrate the OCR function to a new CI setup.
- Configuration Changes: The pull request modifies the CI configuration files to include the OCR function in the development environment and exclude it from the production environment.
- Environment Variables: The pull request introduces a
ci-setup.jsonfile to define environment variables (FUNCTIONS_BUCKETandRESULTS_BUCKET) for the OCR function in the CI environment.
Changelog
- .github/config/nodejs-dev.jsonc
- Added
functions/ocr/appto the list of included functions for the development environment CI at line 152.
- Added
- .github/config/nodejs-prod.jsonc
- Removed
functions/ocr/appfrom the list of included functions for the production environment CI at line 93.
- Removed
- functions/ocr/app/ci-setup.json
- Created a new file to define environment variables for the OCR function in the CI environment.
- Defined
FUNCTIONS_BUCKETandRESULTS_BUCKETenvironment variables, both set to$PROJECT_IDat lines 3 and 4.
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 is currently in preview and 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 to provide feedback.
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.
Did you know?
The first OCR systems were developed in the early 20th century, but they were very limited in their capabilities.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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
The pull request migrates the OCR functionality to a new CI setup. The changes involve adding the OCR app to the development configuration and removing it from the production configuration, along with setting up environment variables for the CI environment. Overall, the changes seem reasonable, but there are a few points to consider.
Summary of Findings
- Inconsistent application of CI setup: The OCR app is added to the development CI configuration but removed from the production configuration. This might indicate an issue with the deployment process or testing strategy. It's important to understand why it was removed from production and ensure that it doesn't negatively impact the functionality.
- Environment variable setup in ci-setup.json: The
ci-setup.jsonfile setsFUNCTIONS_BUCKETandRESULTS_BUCKETto$PROJECT_ID. It's crucial to verify that this setup aligns with the intended behavior and that the project ID is correctly configured in the CI environment. Also, it's not clear if these environment variables are used in the code.
Merge Readiness
The pull request introduces changes to the CI configuration and environment variable setup for the OCR functionality. While the changes seem straightforward, it's important to address the identified issues before merging. Specifically, the inconsistency in the CI configuration between development and production environments needs clarification. Additionally, the environment variable setup in ci-setup.json should be verified to ensure it aligns with the intended behavior. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
briandorsey
left a comment
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
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test(see Testing)npm run lint(see Style)GoogleCloudPlatform/nodejs-docs-samples. Not a fork.