feat: add suport for genkit java#4546
Conversation
Summary of ChangesHello @xavidop, 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 introduces expanded project root detection capabilities within the 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
|
There was a problem hiding this comment.
Code Review
This pull request adds support for detecting Java projects by looking for pom.xml and build.gradle files. The changes correctly extend the findProjectRoot function. My review includes two main suggestions for improvement:
- Refactoring the file-checking logic to reduce code repetition and improve maintainability.
- Adding unit tests for the
findProjectRootfunction to ensure the new detection logic is reliable and prevent future regressions.
On a minor note, there's a typo in the pull request title ('suport' should be 'support').
Head branch was pushed to by a user without write access
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the findProjectRoot function to support additional project types, including Java, by searching for a list of project marker files. The change is a good improvement, making the logic cleaner and more extensible. I have one suggestion to further simplify the implementation and improve its efficiency by using Promise.any. Also, there's a small typo in the PR title ('suport' should be 'support').
| const checks = projectMarkers.map((file) => | ||
| fs | ||
| .access(pyproject2Path) | ||
| .access(path.join(currentDir, file)) | ||
| .then(() => true) | ||
| .catch(() => false), | ||
| ]); | ||
| if ( | ||
| packageJsonExists || | ||
| goModExists || | ||
| pyprojectExists || | ||
| pyproject2Exists | ||
| ) { | ||
| .catch(() => false) | ||
| ); | ||
| const results = await Promise.all(checks); | ||
| if (results.some((found) => found)) { | ||
| return currentDir; | ||
| } |
There was a problem hiding this comment.
The current implementation can be made more concise and efficient by using Promise.any. This will resolve as soon as the first project marker file is found, avoiding unnecessary checks for other markers in the same directory. This also simplifies the code. The existing try...catch block will correctly handle the case where no marker files are found in a directory, as Promise.any will reject.
const promises = projectMarkers.map((file) =>
fs.access(path.join(currentDir, file))
);
await Promise.any(promises);
return currentDir;
Description here... Help the reviewer by:
Checklist (if applicable):