Adding workflow for downloading postcode registry#274
Conversation
📝 WalkthroughWalkthroughAdds a Node.js script to download, parse, and index the Norwegian postal code registry into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/update-postcodes.yml:
- Around line 28-37: The "Check for changes" step (id: changes) uses git diff
--quiet which returns no change for an untracked/new postcodes/registry.json;
update the run logic to detect new or modified files by checking git status
--porcelain for postcodes/registry.json (or use git ls-files --error-unmatch to
detect tracked vs untracked) and write "changed=true" to GITHUB_OUTPUT when git
status returns a non-empty result, otherwise write "changed=false".
🧹 Nitpick comments (2)
.github/scripts/download-postcodes.js (1)
16-21: Consider adding a timeout for fetch reliability.The external API could hang or respond slowly, causing the CI job to wait indefinitely. Consider adding a timeout using
AbortSignal.timeout():♻️ Suggested improvement
async function downloadData() { console.log(`Downloading from ${DATA_URL}...`); - const response = await fetch(DATA_URL); + const response = await fetch(DATA_URL, { + signal: AbortSignal.timeout(30000), // 30 second timeout + }); if (!response.ok) { throw new Error(`Failed to download: ${response.status} ${response.statusText}`); }.github/workflows/update-postcodes.yml (1)
39-52: LGTM! Well-configured automated PR creation.The PR action is properly configured with appropriate commit message, descriptive body, and branch cleanup. The conditional execution based on change detection ensures PRs are only created when needed.
Optional enhancement: Consider adding labels (e.g.,
automated,dependencies) to help filter/identify automated PRs:labels: automated, postcodes
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/scripts/download-postcodes.js.github/workflows/update-postcodes.yml
🔇 Additional comments (5)
.github/scripts/download-postcodes.js (4)
16-26: LGTM! Proper error handling and encoding support.The download function correctly handles HTTP errors and uses the appropriate
windows-1252decoder for ANSI-encoded Norwegian text, which will properly handle special characters likeØ,Æ, andÅ.
28-45: LGTM! Robust parsing with proper validation.Good defensive coding: validates tab-separated format, trims whitespace, and enforces 4-digit zip code format with regex. This handles malformed lines gracefully.
47-64: LGTM! Efficient indexed storage design.The approach of deduplicating place names with 1-based indexing and using a fixed 10000-element mapping array for direct zip-to-index lookup is space-efficient and provides O(1) lookup time. The design is well-suited for serving as a static CDN asset.
66-90: LGTM! Clean orchestration and error handling.The main function properly orchestrates the workflow with appropriate logging, and the entry point handles both success and failure cases with correct exit codes for CI integration.
.github/workflows/update-postcodes.yml (1)
1-14: LGTM! Appropriate workflow configuration.The schedule, manual dispatch trigger, and permissions are well-configured. The
contents: writeandpull-requests: writepermissions follow least-privilege principles for this use case.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/update-postcodes.yml:
- Around line 28-37: The step with id "changes" has inverted logic: the test if
[ -n "$(git status --porcelain postcodes/registry.json)" ] is true when there
ARE changes, but the script currently sets changed=false; swap the outputs so
that when the test is true you echo "changed=true" and when false you echo
"changed=false" (update the two echo lines inside the if/else in the "Check for
changes" step that call "$GITHUB_OUTPUT" and the corresponding user messages).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/update-postcodes.yml
🔇 Additional comments (1)
.github/workflows/update-postcodes.yml (1)
39-52: PR creation configuration looks good.The step correctly gates on the change detection output and uses appropriate settings for automated PRs (descriptive branch name, auto-delete branch, clear commit message and body).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
tjololo
left a comment
tjololo
left a comment
There was a problem hiding this comment.
Don't know to much about the wanted format and can't do a full review of the js code but all in all LGTM.
One comment to eval on the action side
2 participants
Description
This adds a workflow that downloads the latest postcode changes from Posten/Bring and creates a pull request when there are changes. By serving a static file on CDN instead of relying on the Posten/Bring API, we reduce the risk of breaking apps like what happened yesterday and the day before when this API stopped working.
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.