-
Notifications
You must be signed in to change notification settings - Fork 15
fix(time): return both UTC and local time from time tools #52
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
- getCurrentDate and getCurrentTime now return {utc, local, timeZone}
- UTC values remain available for calendar/API integrations
- Local values provide user's actual local time for display
- Updated tool descriptions to explain the dual-format response
- Updated tests and documentation
Fixes #44
Summary of ChangesHello @allenhutchison, 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 refines the time utility functions to provide a more user-centric experience by correctly handling local time. Previously, these functions only returned UTC, which could lead to confusion for users. The update ensures that applications can now display time accurately according to the user's local timezone while still retaining UTC for backend or API compatibility. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully addresses the issue of providing local time alongside UTC time from the time tools. The changes are well-documented and the tool descriptions have been updated accordingly. My review includes suggestions to improve code maintainability by refactoring duplicated code in TimeService.ts and to strengthen the tests in TimeService.test.ts with more specific assertions.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Extract duplicated now/timeZone logic into getTimeContext helper - Strengthen test assertions with format validation (regex)
Summary
This PR fixes issue #44 where time functions were returning UTC time instead of the user's local time.
Problem
The
getCurrentDateandgetCurrentTimefunctions usednew Date().toISOString()which always returns UTC time. This caused incorrect time to be displayed to users in non-UTC timezones.Solution
Both functions now return a response with three fields:
utc: The UTC time (preserved for calendar API compatibility)local: The user's local time (for display purposes)timeZone: The user's timezone (for context)Example response from
getCurrentTime:{ "utc": "01:22:21", "local": "17:22:21", "timeZone": "America/Los_Angeles" }Changes
TimeService.ts: UpdatedgetCurrentDateandgetCurrentTimeto return{utc, local, timeZone}index.ts: Updated tool descriptions to explain the dual-format responseTimeService.test.ts: Updated tests for the new response formatdocs/index.md: Updated documentationTesting
All 330 tests pass.
Fixes #44