Conversation
There was a problem hiding this comment.
Hello @ryoppippi, 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
Hello! Gemini here, ready to provide a summary of this pull request to help everyone quickly understand the changes. This PR, authored by ryoppippi, focuses on improving the code structure by explicitly marking internal modules and preventing them from being exported. This is achieved by introducing a .internal.ts suffix for files intended for internal use, updating all references to these files throughout the codebase, and modifying the build configuration to exclude these internal files. Additionally, the utils export is removed from package.json, reinforcing its status as an internal module.
Highlights
- Code Restructuring with
.internal.ts: Files intended for internal use, such assrc/shared-args.tsandsrc/utils.test.ts, have been renamed to include the.internal.tssuffix (e.g.,src/shared-args.internal.ts). This provides a clear visual indicator of their internal nature. - Import Path Updates: All files that previously imported from the renamed or now-internal modules have been updated to use the new
.internal.tspaths. This includes updates in command files (daily.ts,mcp.ts,monthly.ts,session.ts), data loading (data-loader.ts), and the MCP server implementation (mcp.ts). Imports for types and shared arguments are now pointing to their respective.internal.tsfiles. - Build Configuration Update: The
tsdown.config.tsfile has been updated to exclude all files ending with.test.tsor.internal.tsfrom the build output. This ensures that internal implementation details and test files are not included in the final distribution. - Removed Package Export: The
./utilsexport has been removed from theexportsfield inpackage.json. This explicitly prevents external consumers from importing modules from theutilspath, as it is now considered an internal part of the package.
Changelog
Click here to see the changelog
- package.json
- Removed the
./utilsexport from theexportsfield (line 25).
- Removed the
- src/commands/daily.ts
- Updated import path for
sharedCommandConfigfrom../shared-args.tsto../shared-args.internal.ts(line 13). - Updated import path for
formatCurrencyandformatNumberfrom../utils.tsto../utils.internal.ts(line 14).
- Updated import path for
- src/commands/mcp.ts
- Updated import path for
sharedArgsfrom../shared-args.tsto../shared-args.internal.ts(line 4).
- Updated import path for
- src/commands/monthly.ts
- Updated import path for
sharedCommandConfigfrom../shared-args.tsto../shared-args.internal.ts(line 13). - Updated import path for
formatCurrencyandformatNumberfrom../utils.tsto../utils.internal.ts(line 14).
- Updated import path for
- src/commands/session.ts
- Updated import path for
sharedCommandConfigfrom../shared-args.tsto../shared-args.internal.ts(line 13). - Updated import path for
formatCurrencyandformatNumberfrom../utils.tsto../utils.internal.ts(line 14).
- Updated import path for
- src/data-loader.ts
- Updated import path for
CostModeandSortOrdertypes from./types.tsto./types.internal.ts(line 1).
- Updated import path for
- src/mcp.ts
- Updated import path for
CostModetype from./typesto./types.internal.ts(line 2). - Updated import path for
CostModesanddateSchemafrom./typesto./types.internal.ts(line 12).
- Updated import path for
- src/shared-args.ts -> src/shared-args.internal.ts
- File renamed from
src/shared-args.tstosrc/shared-args.internal.ts. - Updated import path for
CostModeandSortOrdertypes from./typesto./types.internal.ts(line 2). - Updated import path for
CostModes,dateSchema, andSortOrdersfrom./typesto./types.internal.ts(line 5).
- File renamed from
- src/utils.test.ts -> src/utils.internal.test.ts
- File renamed from
src/utils.test.tstosrc/utils.internal.test.ts. - Updated import path for
formatCurrencyandformatNumberfrom./utils.tsto./utils.internal.ts(line 2).
- File renamed from
- tsdown.config.ts
- Updated the
entryglob pattern to exclude files matching**/*.{test,internal}.ts, replacing specific exclusions fortypes.ts,shared-args.ts, and**/*.test.ts(line 6).
- Updated the
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.
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. ↩
commit: |
There was a problem hiding this comment.
Pull Request Overview
This PR restructures the codebase to clearly separate public and internal modules, updates build configuration, and removes an unused export.
- Renames and marks internal modules with
.internal.tssuffix, updating all imports accordingly - Updates
tsdown.config.tsto exclude both.internal.tsand.test.tsfiles from build output - Removes the now-internal
utilsexport frompackage.json
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tsdown.config.ts | Updated glob to exclude .internal.ts and .test.ts |
| src/utils.internal.test.ts | Adjusted test import to use .internal.ts utilities |
| src/shared-args.internal.ts | Switched imports to reference types.internal.ts |
| src/mcp.ts | Switched imports to use types.internal.ts |
| src/data-loader.ts | Switched types import to types.internal.ts |
| src/commands/session.ts | Updated imports for internal shared-args and utils |
| src/commands/monthly.ts | Updated imports for internal shared-args and utils |
| src/commands/mcp.ts | Updated import to use internal shared-args |
| src/commands/daily.ts | Updated imports for internal shared-args and utils |
| package.json | Removed public utils export |
There was a problem hiding this comment.
Code Review
This pull request effectively restructures the codebase to delineate internal modules using the .internal.ts suffix and updates the build configuration accordingly. The changes are clear, consistent, and well-described, leading to improved code organization and a more explicit public API.
Key positive aspects:
- Clear Convention: The introduction and consistent use of the
.internal.tssuffix make the visibility of modules immediately apparent. - Build Configuration: The update to
tsdown.config.tsto exclude*.internal.tsand*.test.tsfiles using a simplified glob pattern (!./src/**/*.{test,internal}.ts) is a clean way to ensure these internal modules are not part of the distributed package. - API Surface Reduction: Removing the
utilsexport frompackage.jsonand ensuring internal modules are not bundled correctly reduces the public API surface, aligning with the goal of better encapsulation. - Consistency: All file renames and import path updates have been applied consistently across the affected files.
The changes significantly enhance the maintainability and clarity of the codebase by making a clear distinction between public and internal components. Well done!
Summary of Findings
- Code Quality and Refactoring: No issues of
medium,high, orcriticalseverity were identified in this pull request. The refactoring to introduce an.internal.tsconvention for internal modules has been executed thoroughly and correctly. This improves code organization and clearly defines the public API of the package. - Build Configuration: The
tsdown.config.tschanges correctly and succinctly ensure that new internal modules (and test files) are excluded from the build output. This is crucial for achieving the PR's goal of not exporting internal modules.
Merge Readiness
The pull request is in excellent shape and appears ready for merging. The changes are well-implemented and achieve the stated goals of improving code organization and clarifying the separation between public and internal modules. As a language model, I am not authorized to approve pull requests, so please ensure it undergoes the standard review and approval process by authorized team members.
stop exporting internal modules
This pull request focuses on restructuring the codebase by introducing the
.internal.tssuffix for files intended for internal use and updating references accordingly. Additionally, it removes theutilsexport frompackage.json. These changes aim to improve code organization and clarify the separation between public and internal modules.Code restructuring:
.internal.tssuffix for internal modulessrc/shared-args.tstosrc/shared-args.internal.tsand updated all references tosharedCommandConfigandsharedArgsinsrc/commands/daily.ts,src/commands/mcp.ts,src/commands/monthly.ts, andsrc/commands/session.ts. [1] [2] [3] [4]src/utils.test.tstosrc/utils.internal.test.tsand updated the imports in the renamed file.CostMode,SortOrder,CostModes, anddateSchemato usetypes.internal.tsinstead oftypes.tsin multiple files (src/data-loader.ts,src/mcp.ts,src/shared-args.internal.ts). [1] [2] [3] [4]Configuration updates
tsdown.config.tsto exclude both.test.tsand.internal.tsfiles from the build output.Removal of unused exports
utilsexport frompackage.json, as it is now considered an internal module.