P5a: idiomatic-Crystal cleanups - #28
Merged
Merged
Conversation
A batch of pre-1.0 idiomatic fixes (kept as one commit — the changes are
entangled across session.cr / v3/session.cr):
- varbind: forward_missing_to value (drop the method_missing splat that swallowed
blocks and defeated type-checking)
- varbind: extract VarBind.from_value(oid, value), deduplicating the set case
shared by Session and V3::Session
- snmp: REQUEST_ID_RANGE constant replacing the repeated rand(2147483647) literal
- v3/message, v3/security: name the magic numbers (MAX_MESSAGE_SIZE,
PASSKEY_EXPANSION_SIZE/PASSKEY_CHUNK_SIZE)
- session, v3/security/{aes,des}: raise typed SNMP::Error/ParseError, not strings
- v3/session: replace deprecated Time.monotonic with Time.instant (timeliness
now a nilable Time::Instant; RFC 3414 window semantics unchanged)
- client, snmp_spec: UDPSocket#read_timeout takes a Time::Span (deprecation)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
64 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Most of the P5 (idiomatic Crystal) group from the audit (#7). Kept as a single commit — the changes are entangled at the file level (e.g.
session.cr/v3/session.creach carry three independent idiomatic changes), so a clean one-commit-per-item split would need heavy per-hunk surgery for little reviewer benefit.Changes
VarBind#method_missing→forward_missing_to value— the old splat macro swallowed blocks and defeated type-checking.VarBind.from_value(oid, value)— deduplicates thesetcasethat was copied verbatim intoSessionandV3::Session.REQUEST_ID_RANGEconstant — replaces therand(2147483647)magic literal repeated across pdu / message / session / v3 (× many).MAX_MESSAGE_SIZE(65507),PASSKEY_EXPANSION_SIZE/PASSKEY_CHUNK_SIZE.raise "string"—SNMP::Error/SNMP::ParseErrorinsession.crand the AES/DES salt checks.Time.monotonic→Time.instant(timeliness is now a nilableTime::Instant; the RFC 3414 window semantics are unchanged, verified bytimeliness_spec);UDPSocket#read_timeoutnow takes aTime::Span.Enum.from_value?(nilable decoding) was already handled by the strict-parse work (#13). The Trap class-hierarchy smell is intentionally deferred (see #7).Tests
Behaviour-preserving refactors, guarded by the full suite (incl. the RFC 3414 passkey vectors and the SEC-3
timeliness_spec). No new deprecation warnings. Deterministic 82/82, multi-threaded 82/82, legacy 3/3, ameba clean, format clean.Tracked in #7.
🤖 Generated with Claude Code