Skip to content

Export InsertJSON func#167

Merged
blakeyjason merged 1 commit intomainfrom
ikolesn/export-insert-json
May 28, 2025
Merged

Export InsertJSON func#167
blakeyjason merged 1 commit intomainfrom
ikolesn/export-insert-json

Conversation

@ikolesn
Copy link
Contributor

@ikolesn ikolesn commented May 28, 2025

No description provided.

@ikolesn ikolesn requested a review from Copilot May 28, 2025 16:52
Copy link

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 exposes the previously private insertJSON method as a public InsertJSON function and updates its call site to supply a full file path.

  • Renamed insertJSON (method on Writer) to InsertJSON (top-level function) and removed its receiver
  • Updated the call site in WriteGeoIP2TestDB to use InsertJSON with filepath.Join(w.source, …)
  • Adjusted function signature and doc comment to match the exported name
Comments suppressed due to low confidence (2)

pkg/writer/geoip2.go:89

  • [nitpick] The parameter name jsonFilePath is a bit verbose; consider renaming it to filePath for conciseness since the function already indicates JSON context.
func InsertJSON(dbWriter *mmdbwriter.Tree, jsonFilePath string) error {

pkg/writer/geoip2.go:74

  • [nitpick] Exporting InsertJSON makes it part of the public API. Verify that this export is intended for external consumption; if not, consider keeping it unexported.
if err := InsertJSON(dbWriter, filepath.Join(w.source, jsonFileName)); err != nil {

Comment on lines +87 to 88
// InsertJSON reads and parses a json file into mmdbtypes values and inserts
// them into the mmdbwriter tree.
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The doc comment for InsertJSON is split across two lines. Combine into one sentence that starts with the function name and ends with a period, e.g., // InsertJSON reads and parses a JSON file into mmdbtypes values and inserts them into the mmdbwriter tree.

Suggested change
// InsertJSON reads and parses a json file into mmdbtypes values and inserts
// them into the mmdbwriter tree.
// InsertJSON reads and parses a JSON file into mmdbtypes values and inserts them into the mmdbwriter tree.

Copilot uses AI. Check for mistakes.
@ikolesn ikolesn force-pushed the ikolesn/export-insert-json branch from c69e7c5 to 9b9c5b7 Compare May 28, 2025 17:20
@blakeyjason blakeyjason merged commit 1485e1d into main May 28, 2025
9 checks passed
@blakeyjason blakeyjason deleted the ikolesn/export-insert-json branch May 28, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants