Skip to content

fix: make dead_lettering.cr compile standalone - #2172

Open
viktorerlingsson wants to merge 1 commit into
mainfrom
fix/dead-lettering-standalone-compile
Open

fix: make dead_lettering.cr compile standalone#2172
viktorerlingsson wants to merge 1 commit into
mainfrom
fix/dead-lettering-standalone-compile

Conversation

@viktorerlingsson

@viktorerlingsson viktorerlingsson commented Aug 7, 2026

Copy link
Copy Markdown
Member

The CI job that compiles each changed *.cr file independently has been failing on src/lavinmq/amqp/argument/dead_lettering.cr:

Error: undefined constant AMQP::Queue

The file references AMQP::Queue in alias Task = {AMQP::Queue, Message} | MessageRoutedCallback but never requires it. It only builds as part of the full program because queue.cr requires it.

Requiring ../queue/queue at the top doesn't work - queue.cr does include Argument::DeadLettering, and an include needs the module body already processed, so entering from dead_lettering.cr just fails the other way (undefined constant Argument::DeadLettering). Requiring it after the module is defined satisfies both entry points: the in-progress require is a no-op on the second pass, and the alias target resolves in the later semantic phase.

Note this wasn't introduced by 8256eff - the file already failed standalone before it. That commit was just the first to touch the file after the changed-files check landed, which is where it surfaced.

Test plan

  • crystal build --no-codegen --error-on-warnings src/lavinmq/amqp/argument/dead_lettering.cr passes
  • Same sweep over every src/**/*.cr - this was the only failure repo-wide
  • crystal build --no-codegen --error-on-warnings src/lavinmq.cr, make lint, crystal tool format --check src/ all clean
  • make test green

🤖 Generated with Claude Code

`dead_lettering.cr` references `AMQP::Queue` in the `Task` alias but never
required it, so the CI job that compiles each changed `*.cr` file on its own
failed on this file.

Requiring `../queue/queue` at the top doesn't work: `queue.cr` does
`include Argument::DeadLettering`, and an `include` needs the module body
already processed, so entering from this file fails the other way around.
Requiring it after the module is defined satisfies both entry points - the
in-progress require is a no-op on the second pass, and the alias target
resolves in the later semantic phase.
@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

No issues found.

The change adds require "../queue/queue" at the end of dead_lettering.cr to break a circular require (queue.cr:18 requires dead_lettering, and DeadLettering references AMQP::Queue). Placing the require after the module definition is the correct idiom for this — Crystal processes each require once, so it works whether dead_lettering.cr or queue.cr is the compilation entry point. The path resolves correctly. No behavioral change, so no spec is warranted.

@viktorerlingsson
viktorerlingsson marked this pull request as ready for review August 7, 2026 11:18
@viktorerlingsson
viktorerlingsson requested a review from a team as a code owner August 7, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant