Skip to content

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

Merged
merged 4 commits into from
Jun 28, 2025
Merged

Prepare for beta 2 #132

merged 4 commits into from
Jun 28, 2025

Conversation

oschwald
Copy link
Owner

  • Replace IsZero with HasData methods
  • Extract model structs to separate file
  • Replace anonymous structs with named types
  • Fix latitude/longitude pointer handling

@oschwald oschwald requested a review from Copilot June 28, 2025 18:08
Copilot

This comment was marked as outdated.

oschwald added 4 commits June 28, 2025 11:21
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.
Copy link

@Copilot Copilot AI left a 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)
Copy link
Preview

Copilot AI Jun 28, 2025

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.

Suggested change
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.

@oschwald oschwald merged commit 32fa95b into main Jun 28, 2025
11 checks passed
@oschwald oschwald deleted the greg/beta-2 branch June 28, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant