fix: merge duplicate imports in packages/core (3/4)#19791
Closed
Nixxx19 wants to merge 13 commits intogoogle-gemini:mainfrom
Closed
fix: merge duplicate imports in packages/core (3/4)#19791Nixxx19 wants to merge 13 commits intogoogle-gemini:mainfrom
Nixxx19 wants to merge 13 commits intogoogle-gemini:mainfrom
Conversation
Merged duplicate import statements across all files in packages/core to comply with the import/no-duplicates ESLint rule. Changes include: - Combined type and value imports from the same module - Merged namespace imports with named imports where appropriate - Fixed import ordering to separate value imports from type imports - Ensured all imports follow the inline type import pattern All 265 test suites pass successfully. Related to google-gemini#19753
Contributor
Summary of ChangesHello @Nixxx19, 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 significantly refactors the import statements across numerous files within the Highlights
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}
Contributor
There was a problem hiding this comment.
Code Review
The pull request effectively merges duplicate import statements across numerous files in packages/core, combining type and value imports using inline type syntax. It also addresses import ordering to comply with the import/no-duplicates ESLint rule. The changes improve code clarity and maintainability by reducing redundancy in import declarations.
Comment on lines
39
to
46
| import { | ||
| type LocalAgentDefinition, | ||
| type AgentInputs, | ||
| type OutputObject, | ||
| type SubagentActivityEvent, | ||
| AgentTerminateMode, | ||
| DEFAULT_QUERY_STRING, | ||
| DEFAULT_MAX_TURNS, |
Contributor
There was a problem hiding this comment.
The type imports for LocalAgentDefinition, AgentInputs, OutputObject, and SubagentActivityEvent are correctly merged with the AgentTerminateMode value import from ./types.js.
import {
type LocalAgentDefinition,
type AgentInputs,
type OutputObject,
type SubagentActivityEvent,
AgentTerminateMode,
DEFAULT_QUERY_STRING,
DEFAULT_MAX_TURNS,
} from './types.js';
Comment on lines
+9
to
13
| type CaGenerateContentResponse, | ||
| toGenerateContentRequest, | ||
| fromGenerateContentResponse, | ||
| toContents, | ||
| } from './converter.js'; |
Contributor
There was a problem hiding this comment.
The CaGenerateContentResponse type import is correctly merged with other value imports from ./converter.js, enhancing import consolidation.
import {
type CaGenerateContentResponse,
toGenerateContentRequest,
fromGenerateContentResponse,
toContents,
} from './converter.js';
Comment on lines
14
to
21
| import { | ||
| GenerateContentResponse, | ||
| FinishReason, | ||
| BlockedReason, | ||
| type ContentListUnion, | ||
| type GenerateContentParameters, | ||
| type Part, | ||
| } from '@google/genai'; |
Contributor
There was a problem hiding this comment.
This change correctly merges the GenerateContentResponse, FinishReason, and BlockedReason value imports with the ContentListUnion, GenerateContentParameters, and Part type imports from @google/genai, reducing import redundancy.
import {
GenerateContentResponse,
FinishReason,
BlockedReason,
type ContentListUnion,
type GenerateContentParameters,
type Part,
} from '@google/genai';
Comment on lines
7
to
13
| import { | ||
| type Credentials, | ||
| type AuthClient, | ||
| type JWTInput, | ||
| OAuth2Client, | ||
| Compute, | ||
| CodeChallengeMethod, |
Contributor
There was a problem hiding this comment.
This change correctly merges the Credentials, AuthClient, and JWTInput type imports with several value imports from google-auth-library, reducing import redundancy.
import {
type Credentials,
type AuthClient,
type JWTInput,
OAuth2Client,
Compute,
CodeChallengeMethod,
} from 'google-auth-library';
Comment on lines
41
to
46
| import { | ||
| type CaCountTokenResponse, | ||
| type CaGenerateContentResponse, | ||
| fromCountTokenResponse, | ||
| fromGenerateContentResponse, | ||
| toCountTokenRequest, |
Contributor
There was a problem hiding this comment.
The CaCountTokenResponse and CaGenerateContentResponse type imports are correctly merged with other value imports from ./converter.js, enhancing import consolidation.
import {
type CaCountTokenResponse,
type CaGenerateContentResponse,
fromCountTokenResponse,
fromGenerateContentResponse,
toCountTokenRequest,
} from './converter.js';
gemini-cli
bot
added
the
area/core
This was referenced Feb 21, 2026
Nixxx19
changed the title
Nixxx19
force-pushed
the
nityam/cleanup-import-no-duplicates-core
branch
from
20669cc to
51e6309
Compare
Resolved conflict in packages/core/src/config/config.test.ts: - Added type import for ContentGenerator (needed for type annotations) - ContentGeneratorConfig already imported as type in regular import, so no duplicate needed
Separate type import for ContentGenerator from regular imports to maintain proper TypeScript import structure
gemini-cli
bot
added
priority/p2
scidomino
requested changes
Feb 27, 2026
Collaborator
|
please resolve your merge conflict |
Resolved conflicts in: - packages/core/src/code_assist/converter.ts: Added debugLogger and Credits type imports - packages/core/src/code_assist/server.ts: Merged type imports, added billing imports and GeminiUserTier - packages/core/src/config/config.ts: Added OverageStrategy import - packages/core/src/scheduler/scheduler_parallel.test.ts: Added GeminiCliOperation import - packages/core/src/scheduler/tool-executor.ts: Added telemetry constants imports - packages/core/src/tools/mcp-client.ts: Split imports and added GeminiCLIExtension - packages/core/src/tools/write-file.test.ts: Kept EditToolParams and editCorrector imports with eslint-disable - packages/core/src/utils/editCorrector.test.ts: Simplified vitest imports
Addresses reviewer feedback: The currentMode variable was already retrieved, so we should use it consistently rather than calling getApprovalMode() again. This ensures consistency and avoids potential issues if the function returns different values on subsequent calls.
Since there's no name conflict after import cleanup, we can use the standard 'path' import name instead of 'nodePath'.
This line was accidentally added during merge conflict resolution but was not present in origin/main. Removing it to keep the PR focused on import cleanup only.
Contributor
Author
|
@scidomino made all the changes and fixed conflicts too, thanks for catching! |
Collaborator
|
I'm sorry. There are more merge conflicts. My advice is to break this up into smaller sections. That will make it easier to review and easier to resolve conflicts. |
Collaborator
|
I see you resolved them. Ok. Will review now. |
scidomino
reviewed
Mar 2, 2026
Collaborator
scidomino
left a comment
scidomino
left a comment
There was a problem hiding this comment.
More unrelated edits probably due to a bad merge. I stopped reviewing after I saw that file so please do check the rest to make sure they aren't also affected
| let mockGeminiClientInstance: Mocked<GeminiClient>; | ||
| let mockBaseLlmClientInstance: Mocked<BaseLlmClient>; | ||
| let mockConfig: Config; | ||
| const mockEnsureCorrectEdit = vi.fn<typeof ensureCorrectEdit>(); |
Collaborator
There was a problem hiding this comment.
These changes (and the others in this file) seem unconnected to imports.
| mockEnsureCorrectFileContent.mockReset(); | ||
|
|
||
| // Default mock implementations that return valid structures | ||
| mockEnsureCorrectEdit.mockImplementation( |
Contributor
Author
i will create a fresh new PR with the fix, sorry about that, thanks for flagging! |
Contributor
Author
|
hey @scidomino, made a new fresh pr with no merge conflicts #20928, it got closed by the bot, can you please reopen and review it, thanks!! |
|
You already have 7 pull requests open. Please work on getting existing PRs merged before opening more. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/coretypesyntaximport/no-duplicatesESLint ruleChanges
--fixwithimport/no-duplicatesrule to automatically merge most duplicatesgetFolderStructure.test.tsto use correct namespace import referencesTesting
Stats
Related
Fixes #19753 - PR 3 of 4 (packages/core)
Previous PRs: