-
Notifications
You must be signed in to change notification settings - Fork 210
Prepare for beta 2 #132
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
Prepare for beta 2 #132
Conversation
oschwald
commented
Jun 28, 2025
- Replace IsZero with HasData methods
- Extract model structs to separate file
- Replace anonymous structs with named types
- Fix latitude/longitude pointer handling
Replace IsZero() methods with HasData() methods on all result structs. HasData() provides clearer semantics and excludes Network/IPAddress fields from validation, allowing access to network information even when no GeoIP data is found.
Move all model structs (Names, Enterprise, City, Country, etc.) and their HasData methods from reader.go to a new models.go file. This improves code organization by separating data models from the Reader implementation.
* Replace all anonymous nested structs with named types * Eliminate RegisteredCountry duplication, use CountryRecord * Optimize HasData methods with zero struct comparison * Simplify parent HasData methods using substruct methods This improves struct initialization ergonomics while maintaining the same JSON serialization behavior and performance.
Change Location.Latitude and Location.Longitude from float64 to *float64 to distinguish missing coordinates from valid (0,0) location. Add HasCoordinates() method for safe coordinate access. Closes #5.
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.
Pull Request Overview
This PR updates the API to replace the IsZero() methods with HasData() methods across result structs, extracts model structs to a separate file, replaces anonymous structs with named types, and fixes latitude/longitude pointer handling.
- Replaces IsZero() with HasData() to clarify data presence checks.
- Extracts model definitions and updates struct types for improved ergonomics.
- Adjusts coordinate handling by changing float64 fields to *float64 and adding HasCoordinates().
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
reader_test.go | Updates assertions to reflect new HasData() semantics and pointer handling for coordinates. |
reader.go | Removes redundant conditional checks since Network/IPAddress fields are now always set. |
models.go | Extracts model definitions with new named types and HasData() methods. |
example_test.go | Updates examples to use HasData() and safe coordinate access. |
README.md | Revises documentation to describe HasData() and coordinate handling changes. |
CHANGELOG.md | Document breaking changes in API and type updates. |
Comments suppressed due to low confidence (2)
reader.go:181
- Since the removal of the 'if result.Found()' block implies that IPAddress and Network fields are always set, please ensure that result.Prefix() returns a valid default when no data is found.
enterprise.Traits.IPAddress = ipAddress
README.md:14
- Verify that the documentation clearly explains the new HasData() semantics compared to the old IsZero() behavior for accurate user understanding.
`IPAddress` fields to all result structs, and includes a `HasData()` method to
@@ -79,8 +79,8 @@ func TestReader(t *testing.T) { | |||
assert.Equal(t, expectedCountryNames, record.Country.Names) | |||
|
|||
assert.Equal(t, uint16(100), record.Location.AccuracyRadius) |
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.
Consider adding an explicit check using record.Location.HasCoordinates() before dereferencing Latitude and Longitude pointers to prevent potential nil pointer dereferences.
assert.Equal(t, uint16(100), record.Location.AccuracyRadius) | |
assert.Equal(t, uint16(100), record.Location.AccuracyRadius) | |
if !record.Location.HasCoordinates() { | |
t.Fatalf("Location does not have valid coordinates") | |
} |
Copilot uses AI. Check for mistakes.