fix: (some minor improvements to configs and getPackageJson return behaviour) - #12510
fix: (some minor improvements to configs and getPackageJson return behaviour)#12510scidomino merged 19 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @grMLEqomlkkU5Eeinz4brIrOVCUCkJuN, 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 focuses on minor code quality enhancements identified during a code review. The changes improve the maintainability and clarity of the codebase by removing redundancies and adding essential documentation, all without introducing any functional alterations. 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.
Code Review
This pull request introduces a set of minor but positive code quality enhancements. The changes, including removing a duplicate ESLint ignore pattern, eliminating a redundant import, and improving documentation for getPackageJson, are all well-executed and improve the maintainability of the codebase. The changes are correct and I have no further feedback.
|
Ah... welp do i need to make changes @scidomino ? |
Head branch was pushed to by a user without write access
|
Please resolve the lint error. I don't understand what you are doing to CONTRIBUTING.md but somehow it's being flagged. |
I removed a "return" character that was somehow added in there (looking at my command line history it seems that after running a linter it added that there) sorry for the headaches |
|
I still see the lint error. There is a way to run the linter locally I think. Alternatively, just revert the changes to that shortcut entirely. |
Sorry, I had 0 clue why that was happening. I apologize for the troubles |
|
The error is in |
Right away, Also I found the error, I clone it on windows and somehow the file stopped becoming a symbolic link, it wasn't the linter at all (the reason why i thought it was the linter was because I noticed that somehow after running prettier, it was the last time git tracked it as the correct file) |
|
git refuses to let me restore the symlink, even when restoring the commit directly. (windows isue?) |
Head branch was pushed to by a user without write access
This reverts commit 18f4373.
|
I removed it. Merging now. |
Sorry, mate 😢I hope I didn't cause too much trouble. |
No worries. Git and windows caused the trouble, not you. |
Merge latest upstream changes from google-gemini/gemini-cli main branch. ## Merge Conflict Resolution **File:** packages/core/src/services/chatCompressionService.ts **Conflict:** Both branches modified the compression service API: - main: Migrated to new model config system (getBaseLlmClient, modelConfigKey) - feat/deliberate-compaction: Added goal-focused compression (userGoal parameter) **Resolution:** Combined both changes: - Used new model config API from main (getBaseLlmClient, modelStringToModelConfigAlias, abortSignal) - Preserved goal-focused enhancement (getChatCompressionPrompt(userGoal)) - Result: New API + goal awareness working together **Additional Changes:** - Updated chatCompressionService.test.ts to use new BaseLlmClient API - Changed all test mocks from getContentGenerator to getBaseLlmClient - Added BaseLlmClient import and proper type casts - Fixed linting: changed messagesPreserved/messagesCompressed to const - Build passes successfully ## Upstream Changes Merged From google-gemini/gemini-cli main (95693e2..d14779b): - feat(core): Land bool for alternate system prompt (google-gemini#13764) - feat(hooks): Hook Agent Lifecycle Integration (google-gemini#9105) - feat(hooks): Hook Event Handling (google-gemini#9097) - fix: Minor improvements to configs and getPackageJson (google-gemini#12510) - feat(hooks): Hook Telemetry Infrastructure (google-gemini#9082) - feat(core): Migrate chatCompressionService to model configs (google-gemini#12863) - Add session subtask in /stats command (google-gemini#13750) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…haviour) (#12510) Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
…getPackageJson return behaviour) (google-gemini#12510)
…haviour) (google-gemini#12510) Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
…haviour) (#12510) Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
Summary
This PR fixes three minor things identified during code review:
.integration-tests/**ignore pattern in ESLint configgetPackageJsonreturn behaviorThese are minor cleanup fixes that improve code maintainability without changing functionality.
Details
.integration-tests/**entry from the ignores array (was listed twice on lines 32 and 37)import './src/gemini.js'since we're already importingmainfrom the same filegetPackageJsonexplaining that returningundefinedis expected behavior when no package.json exists in the directory hierarchy, and removed the TODO comment. (like all the chaining is already there, why not just make use of it?)Related Issues
N/A - Just minor improvements
How to Validate
npm run lintnpm run startshould launch the CLI without errorsnpm run build && npm run startshould work in production buildgetPackageJsoncan be called and returnsundefinedwhen appropriatepackages/cli/src/utils/version.ts,packages/core/src/utils/channel.ts) still work correctlyPre-Merge Checklist
npm run lint