Skip to content

Commit 438a867

Browse files
committed
Fix PR review issues: stale docs, error handling, and security concerns
- Update CLAUDE.md with correct Ruby 3.4.8 and rubocop 3.4 target versions - Fix CLAUDE.md architecture description (Source not Transform, add datafile step, namespaces) - Remove redundant .freeze from write_version_file to match frozen_string_literal - Replace Kernel#open with File.read in version.rake - Add logger gem as runtime dependency (leaving default gems in Ruby 4.0) - Add StandardError rescue to CountryTable#write and StateTable#write - Add nil guard for unknown country codes in CountryTable#write - Fix .rubocop.yml section header (Naming Cops, not Metrics Cops)
1 parent 906f30c commit 438a867

7 files changed

Lines changed: 17 additions & 9 deletions

File tree

.rubocop.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ Metrics/AbcSize:
8686
Enabled: false
8787

8888
########################################
89-
# Metrics Cops
89+
# Naming Cops
9090

9191
Naming/FileName:
9292
Enabled: false

CLAUDE.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,38 +45,38 @@ bundle exec rake release
4545
The gem follows an ETL (Extract, Transform, Load) pattern using the Kiba gem:
4646

4747
1. **Extract**: `DataSource` downloads zip files from GeoNames.org, extracts them, and prepares CSV files with headers
48-
2. **Transform**: `CsvSource` (Kiba source) reads rows from the prepared CSV
48+
2. **Source**: `CsvSource` (Kiba source) feeds rows from the prepared CSV into the pipeline
4949
3. **Load**: Four Kiba destination table classes write rows into an in-memory SQLite database
5050

5151
### Key Flow
5252

53-
`bin/free_zipcode_data``Runner#start``DataSource#download``SqliteRam` (in-memory DB) → `ETL::FreeZipcodeDataJob` (Kiba pipeline) → `SqliteRam#save_to_disk`
53+
`bin/free_zipcode_data``Runner#start``DataSource#download``DataSource#datafile` (extract zip + add CSV headers) → `SqliteRam` (in-memory DB) → `ETL::FreeZipcodeDataJob` (Kiba pipeline) → `SqliteRam#save_to_disk`
5454

5555
### Core Classes
5656

5757
- **`FreeZipcodeData::Runner`** - CLI entry point; parses args via Optimist, orchestrates the full pipeline
5858
- **`FreeZipcodeData::DataSource`** - Downloads and extracts GeoNames zip files, prepares CSV with headers
5959
- **`SqliteRam`** - Wraps SQLite3; works entirely in-memory then saves to disk via `SQLite3::Backup`
6060
- **`FreeZipcodeData::DbTable`** - Base class for all table classes; provides progress bar, SQL helpers, and country lookup from `country_lookup_table.yml`
61-
- **`CountryTable`/`StateTable`/`CountyTable`/`ZipcodeTable`** - Kiba destinations; each has `build` (creates schema + indexes) and `write` (inserts rows, swallows duplicate constraint violations)
61+
- **`FreeZipcodeData::CountryTable`/`StateTable`/`CountyTable`/`ZipcodeTable`** - Kiba destinations; each has `build` (creates schema + indexes) and `write` (inserts rows, swallows duplicate constraint violations)
6262
- **`ETL::FreeZipcodeDataJob`** - Configures the Kiba pipeline with one source and four destinations
6363
- **`CsvSource`** - Kiba-compatible CSV reader
6464

6565
### Singletons
6666

67-
`Options` and `Logger` are singletons (via Ruby's `Singleton` module). `Runner` uses a manual singleton pattern.
67+
`Options` and `Logger` are singletons (via Ruby's `Singleton` module). `Runner` has an `.instance` convenience class method (returns `new` each time, not cached).
6868

6969
## Configuration
7070

71-
- `.ruby-version`: 3.0.2
71+
- `.ruby-version`: 3.4.8
7272
- Bundle path: `vendor/bundle` (binstubs in `stubs/`)
7373
- Environment: `APP_ENV` controls environment (`test`, `development`)
7474
- Config file: `~/.free_zipcode_data.yml` (overridable via `FZD_CONFIG_FILE` env var; uses `spec/fixtures/` version in test)
7575

7676
## Rubocop
7777

7878
Key style settings (`.rubocop.yml`):
79-
- Target Ruby 2.7
79+
- Target Ruby 3.4
8080
- Max line length: 110
8181
- Max method length: 30 lines
8282
- `Style/ClassVars`, `Style/Documentation`, `Metrics/AbcSize`, `Lint/SuppressedException` disabled

Gemfile.lock

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ PATH
55
colored (~> 1.2)
66
csv
77
kiba (~> 4.0)
8+
logger
89
optimist (~> 3.0)
910
ruby-progressbar (~> 1.9)
1011
rubyzip (>= 1.2.2)
@@ -23,6 +24,7 @@ GEM
2324
kiba (4.0.0)
2425
language_server-protocol (3.17.0.5)
2526
lint_roller (1.1.0)
27+
logger (1.7.0)
2628
method_source (0.9.2)
2729
mini_portile2 (2.8.9)
2830
optimist (3.2.1)

free_zipcode_data.gemspec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Gem::Specification.new do |spec|
2626
spec.add_dependency 'colored', '~> 1.2'
2727
spec.add_dependency 'csv'
2828
spec.add_dependency 'kiba', '~> 4.0'
29+
spec.add_dependency 'logger'
2930
spec.add_dependency 'optimist', '~> 3.0'
3031
spec.add_dependency 'ruby-progressbar', '~> 1.9'
3132
spec.add_dependency 'rubyzip', '>= 1.2.2'

lib/free_zipcode_data/country_table.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ def build
2525

2626
def write(row)
2727
country_hash = country_lookup_table[row[:country]]
28+
return update_progress unless country_hash
2829

2930
sql = <<-SQL
3031
INSERT INTO countries (alpha2, alpha3, iso, name)
@@ -38,6 +39,8 @@ def write(row)
3839
database.execute(sql)
3940
rescue SQLite3::ConstraintException
4041
# Swallow duplicates
42+
rescue StandardError => e
43+
raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
4144
end
4245

4346
update_progress

lib/free_zipcode_data/state_table.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ def write(row)
4444
database.execute(sql)
4545
rescue SQLite3::ConstraintException
4646
# Swallow duplicates
47+
rescue StandardError => e
48+
raise "Please file an issue at #{ISSUE_URL}: [#{e}] -> SQL: [#{sql}]"
4749
end
4850

4951
update_progress

lib/tasks/version.rake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ namespace :version do
9191

9292
def write_version_file(version_array)
9393
version = version_array.join('.')
94-
new_version = %( VERSION = '#{version}'.freeze)
94+
new_version = %( VERSION = '#{version}')
9595
lines = File.readlines(version_file_path)
9696
File.open(version_file_path, 'w') do |f|
9797
lines.each do |line|
@@ -106,7 +106,7 @@ namespace :version do
106106

107107
def update_readme_version_strings
108108
version_string = read_version.join('.')
109-
readme = open('README.md').read
109+
readme = File.read('README.md')
110110
regex = /^\*\*Version: [0-9.]+\*\*$/i
111111
return nil unless readme =~ regex
112112

0 commit comments

Comments
 (0)