Replace init() with explicit timezone initialization - #25
Conversation
- Remove global tz variable and init() from util.go - Add location *time.Location field to Client struct - NewClient sets location to America/New_York (UTC fallback) - NewClientWithLocation allows callers to specify a custom timezone - NewCustomClient sets default America/New_York location - parseTime becomes a method on *Client - Add tests for timezone initialization and fallback
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #25 +/- ##
==========================================
+ Coverage 88.25% 88.67% +0.42%
==========================================
Files 5 5
Lines 349 362 +13
==========================================
+ Hits 308 321 +13
Misses 24 24
Partials 17 17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the API client to support configurable timezone locations for parsing timestamps, replacing the previous hardcoded global timezone. The feedback suggests preventing potential nil pointer dereferences in parseTime when the client or its location is nil, and recommends passing *time.Location directly to NewClientWithLocation instead of a string to avoid silent fallbacks and align with Go idioms.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func (c *Client) parseTime(ts string) (time.Time, error) { | ||
| return time.ParseInLocation("02-Jan-2006 03:04:05 PM", ts, c.location) | ||
| } |
There was a problem hiding this comment.
If c or c.location is nil (for example, if a Client is initialized manually without using the constructor functions), calling c.parseTime will cause a nil pointer dereference panic. Adding a defensive check to default to time.UTC ensures safety and robustness.
func (c *Client) parseTime(ts string) (time.Time, error) {
loc := time.UTC
if c != nil && c.location != nil {
loc = c.location
}
return time.ParseInLocation("02-Jan-2006 03:04:05 PM", ts, loc)
}| func NewClient(baseURL, username, password string) *Client { | ||
| return NewClientWithLocation(baseURL, username, password, "America/New_York") | ||
| } | ||
|
|
||
| // NewClientWithLocation constructs a new client with a custom timezone location. | ||
| // The location string must be a valid IANA Time Zone name (e.g. "America/New_York"). | ||
| // If the location fails to load, it falls back to UTC. | ||
| func NewClientWithLocation(baseURL, username, password, location string) *Client { | ||
| loc, err := time.LoadLocation(location) | ||
| if err != nil { | ||
| loc = time.UTC | ||
| } | ||
| return &Client{ | ||
| httpClient: &http.Client{Timeout: 30 * time.Second}, | ||
| baseURL: baseURL, | ||
| username: username, | ||
| password: password, | ||
| location: loc, | ||
| } | ||
| } |
There was a problem hiding this comment.
Accepting a *time.Location directly in NewClientWithLocation is more idiomatic in Go and avoids silent fallbacks to UTC when an invalid timezone string is passed. Silent fallbacks can lead to hard-to-debug issues where times are parsed in UTC instead of the intended timezone due to a typo. Callers can load the location themselves using time.LoadLocation and handle any errors explicitly.
func NewClient(baseURL, username, password string) *Client {
return NewClientWithLocation(baseURL, username, password, defaultLocation())
}
// NewClientWithLocation constructs a new client with a custom timezone location.
// If the provided location is nil, it falls back to UTC.
func NewClientWithLocation(baseURL, username, password string, loc *time.Location) *Client {
if loc == nil {
loc = time.UTC
}
return &Client{
httpClient: &http.Client{Timeout: 30 * time.Second},
baseURL: baseURL,
username: username,
password: password,
location: loc,
}
}| func TestNewClientWithLocation(t *testing.T) { | ||
| c := NewClientWithLocation("http://example.com", "user", "pass", "UTC") | ||
| if c.location != time.UTC { | ||
| t.Errorf("expected UTC location, got %v", c.location) | ||
| } | ||
| } | ||
|
|
||
| func TestNewClientWithLocationFallback(t *testing.T) { | ||
| c := NewClientWithLocation("http://example.com", "user", "pass", "bogus/timezone") | ||
| if c.location != time.UTC { | ||
| t.Errorf("expected UTC fallback for invalid location, got %v", c.location) | ||
| } | ||
| } |
There was a problem hiding this comment.
Update the tests to match the new signature of NewClientWithLocation which accepts a *time.Location directly.
| func TestNewClientWithLocation(t *testing.T) { | |
| c := NewClientWithLocation("http://example.com", "user", "pass", "UTC") | |
| if c.location != time.UTC { | |
| t.Errorf("expected UTC location, got %v", c.location) | |
| } | |
| } | |
| func TestNewClientWithLocationFallback(t *testing.T) { | |
| c := NewClientWithLocation("http://example.com", "user", "pass", "bogus/timezone") | |
| if c.location != time.UTC { | |
| t.Errorf("expected UTC fallback for invalid location, got %v", c.location) | |
| } | |
| } | |
| func TestNewClientWithLocation(t *testing.T) { | |
| c := NewClientWithLocation("http://example.com", "user", "pass", time.UTC) | |
| if c.location != time.UTC { | |
| t.Errorf("expected UTC location, got %v", c.location) | |
| } | |
| } | |
| func TestNewClientWithLocationFallback(t *testing.T) { | |
| c := NewClientWithLocation("http://example.com", "user", "pass", nil) | |
| if c.location != time.UTC { | |
| t.Errorf("expected UTC fallback for nil location, got %v", c.location) | |
| } | |
| } |
2 participants
Fixes #11
tzvariable andinit()from util.golocation *time.Locationfield toClientstructNewClientsets location toAmerica/New_York(UTC fallback on error)NewClientWithLocationallows callers to specify a custom timezoneNewCustomClientuses the default America/New_York locationparseTimebecomes a method on*Client