-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add collector for Wakefield Council #111
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
base: main
Are you sure you want to change the base?
Conversation
Closes #86 Generated with Codex CLI by Moley-Bot
Formatted by Moley-Bot
|
/gemini review |
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.
Code Review
This pull request introduces a new collector for Wakefield Council, complete with integration tests. The implementation correctly follows the multi-step request pattern necessary for this council's website. My review includes several minor suggestions to align the code more closely with the repository's style guide, focusing on code cleanup (like removing an unused using statement), ensuring all external strings are trimmed, and adding standard comments for consistency. I also noted a small opportunity to simplify URL handling.
|
@claude Resolve PR comments, run tests to validate changes, and check if meets style guide and make any appropriate amendments. |
|
Claude finished @BadgerHobbs's task in 6m 33s —— View job Resolved PR Comments and Made Code Improvements ✓SummaryI've successfully addressed all PR review comments from both @gemini-code-assist and @BadgerHobbs. The code has been refactored to improve readability, simplify logic, and align with the repository's style guide. Changes MadeCode Quality Improvements:
Style Guide Compliance:
Code Simplification:
Testing NoteThe integration test encounters an environmental issue with the gov.uk API lookup (separate from the collector implementation). The error occurs in Since the PR description shows successful test output from the original implementation, and our changes only refactor the code for better quality without changing the core logic, the collector should function correctly when the gov.uk API is responsive. Files Modified
--- | |
- Remove unnecessary postcode formatting in GetAddresses - Simplify cookie handling by making it non-conditional - Improve address parsing with better regex that directly captures query parameters - Remove unnecessary usrn (street) and postcode query parameters from GetBinDays - Add .Trim() to service and futureCollections strings - Split date parsing over multiple lines for better readability - Add standard comments before foreach loops per style guide - Add DateTimeStyles.None parameter to DateOnly.ParseExact calls Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
|
@claude Change the postcode to 'WF5 0AF' instead and test with that. |
|
Claude finished @BadgerHobbs's task in 1m 49s —— View job Test Postcode Changed to WF5 0AF ✅Todo List:
SummarySuccessfully changed the test postcode from Changes Made
Test Results✅ Integration test passed successfully with the new postcode: The collector successfully retrieved bin collection data for the WF5 0AF postcode area. | |
Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
BadgerHobbs
left a comment
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.
@claude Fix outstanding/new pr comments.
|
Claude finished @moley-bot[bot]'s task in 2m 55s —— View job PR Review Comments Fixed ✅All three outstanding PR review comments have been successfully addressed and tested. Changes Made1. Removed postcode escaping (line 100)
2. Made cookie handling non-conditional (line 94)
3. Replaced multi-line conditional with if-statement (line 124-131)
Test Results✅ All WakefieldCouncil integration tests passed successfully: Commit DetailsChanges have been committed locally with the following details:
Push StatusUnable to push automatically due to remote branch being updated while working on these changes. The commit is ready and can be pushed manually with: git fetch origin
git rebase origin/collector/WakefieldCouncil-issue-86-1768213716
git push origin collector/WakefieldCouncil-issue-86-1768213716Or if you prefer to force push (since this is a feature branch): git push --force origin collector/WakefieldCouncil-issue-86-1768213716Files Modified
|
- Remove conditional cookie handling - expect set-cookie header to be present - Remove unnecessary postcode escaping in URL parameter - Convert multi-line ternary to if-statement for readability Co-authored-by: Andrew Riggs <BadgerHobbs@users.noreply.github.com>
68cc0a4 to
4a22c7d
Compare
Formatted by Moley-Bot
Summary
This PR adds a new bin collection data collector for Wakefield Council.
ICollectorinterfaceCloses #86
Test Summary
Generated automatically by Moley-Bot using Codex CLI