Add Client.head convenience function#7
Conversation
There was a problem hiding this comment.
Build & Tests
Build: cannot verify locally (pre-existing TM.tm_zone const-correctness issue on this platform — same failure on main). CI passes on both macOS and Ubuntu.
Tests: pass in CI. Three new tests: HEAD returns 200, HEAD returns empty body, head-with-config returns 200.
Findings
1. README not updated (blocking)
The README API table (lines 89-106) lists every convenience function — get, post, put, del, patch, and all their -with-config variants. The new Client.head and Client.head-with-config are not in the table. Users scanning the README won't know HEAD has a shortcut.
Add two rows to the API table:
Client.head url— HTTP HEADClient.head-with-config url config— HEAD with request config
2. Implementation is correct (no issues)
Client.head follows the exact pattern of Client.get and Client.del (no-body methods: pass method string, empty headers, empty body). Client.head-with-config follows Client.get-with-config and Client.del-with-config. Both are placed correctly in the file alongside their pattern counterparts.
3. Tests are appropriate
The old manual Client.request "HEAD" ... test was replaced with the new convenience function — good, no redundant test. The empty-body assertion is a useful HEAD-specific check.
Verdict: revise
The code itself is trivially correct (mechanical pattern-following), but the README API table needs the two new entries to stay complete.
|
Added This addresses the blocking finding from @carpentry-reviewer's review ("README not updated"). |
There was a problem hiding this comment.
Build & Tests
Build: cannot verify locally (no Carp compiler on this platform). CI green on both macOS and Ubuntu.
Tests: pass in CI. Three HEAD tests (200 status, empty body, head-with-config) all passing.
Prior feedback
The blocking finding (README not updated) has been addressed: Client.head and Client.head-with-config are now in the API table alongside the other no-body methods.
Findings
No issues found. The implementation is mechanically correct — head and head-with-config follow the exact same pattern as get/del and their -with-config variants (empty headers map, empty body string, pass-through to request/request-with-config). The README entries are correctly placed and formatted consistently with existing rows. Test coverage is adequate for convenience wrappers over request.
Verdict: merge
Trivially correct pattern-following with proper README documentation. Ready to merge.
Add
Client.headandClient.head-with-configconvenience functions, following the existing pattern ofget/del(no-body methods) and their-with-configvariants.HEAD is a standard HTTP method for header-only checks (e.g. checking if a resource exists, reading Content-Length without downloading the body). Currently it requires a manual
Client.request "HEAD" ...call while all other common methods have shortcuts.Changes
Client.head— takes a URL, returns(Result Response String)Client.head-with-config— same, with aRequestConfigfor timeouts/redirectsClient.request "HEAD"testOpened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.