Skip to content

fix: remove leading blank lines from compileall snapshots#5752

Merged
max-sixty merged 1 commit intomainfrom
fix/compileall-snapshot-blank-lines
Mar 28, 2026
Merged

fix: remove leading blank lines from compileall snapshots#5752
max-sixty merged 1 commit intomainfrom
fix/compileall-snapshot-blank-lines

Conversation

@prql-bot
Copy link
Copy Markdown
Collaborator

Summary

  • Fix the compileall test snapshot generation to skip empty diffs (where a dialect's output matches generic SQL), eliminating leading and intermediate blank lines in .snap files
  • The root cause was that unified_diff() returns an empty string when there's no difference, but those empty strings were still being collected and joined with newlines

Follow-up from #5750 (requested here).

Test plan

  • All compileall snapshots regenerated without blank lines at the top
  • cargo test -p prqlc passes (449 passed, 0 failed)
  • No legacy format warnings from insta

🤖 Generated with Claude Code

Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.

Closes follow-up from #5750.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot prql-bot enabled auto-merge (squash) March 28, 2026 21:12
@max-sixty max-sixty disabled auto-merge March 28, 2026 21:18
@max-sixty max-sixty merged commit bc7099c into main Mar 28, 2026
36 checks passed
@max-sixty max-sixty deleted the fix/compileall-snapshot-blank-lines branch March 28, 2026 21:18
@max-sixty
Copy link
Copy Markdown
Member

@prql-bot look at what happened re the auto-merge

Now that we have the merge protection, we should disable the automerge; can you open a PR for this?

@prql-bot
Copy link
Copy Markdown
Collaborator Author

The automerge job in pull-request-target.yaml ran on this PR — it detected prql-bot as the actor with 1 commit, so it executed gh pr merge --auto --squash --delete-branch, which queued the PR to merge as soon as required checks passed (~21:18 UTC).

Opened #5753 to remove the automerge job entirely now that branch protection handles merge gating.

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.

2 participants