Skip to content

linter: add file-level override to disable assume_in_transaction#996

Merged
kodiakhq[bot] merged 4 commits into
sbdchd:masterfrom
reteps:transaction-file-disable
May 3, 2026
Merged

linter: add file-level override to disable assume_in_transaction#996
kodiakhq[bot] merged 4 commits into
sbdchd:masterfrom
reteps:transaction-file-disable

Conversation

@reteps

@reteps reteps commented Mar 12, 2026

Copy link
Copy Markdown

Summary

  • Adds -- squawk-disable-assume-in-transaction SQL comments to override the global assume_in_transaction setting on a per-file basis
  • Follows the existing squawk-ignore / squawk-ignore-file comment conventions (supports both -- and /* */ styles, plus trailing comments)
  • Useful when a migration tool wraps files in transactions by default but specific files need to opt out (e.g., for CREATE INDEX CONCURRENTLY)

Closes #990

Testing

  • Unit tests for comment parsing (find_transaction_override)
  • Integration test: squawk-disable-assume-in-transaction allows CREATE INDEX CONCURRENTLY when assume_in_transaction is globally enabled
  • Integration test: squawk-disable-assume-in-transaction allows explicit BEGIN/COMMIT when assume_in_transaction is globally enabled

🤖 Generated with Claude Code


Of course, disclaimer, code was written with Claude 4.6 Opus. Feel free to use this PR as a starting point, request changes, or close this out. Thank you!

This is useful to us for the exact reasons I spelled out in #990 -- this will let us turn on more config flags. I changed the comment messages themselves because i felt this better lined up with how the comments worked. I am of course open to other names / ways of configuring on/off.

Add `-- squawk-disable-assume-in-transaction` and `-- squawk-enable-assume-in-transaction`
comments to override the global `assume_in_transaction` setting on a per-file basis.

This is useful when a migration tool wraps files in transactions by default but
specific files need to opt out (e.g., for `CREATE INDEX CONCURRENTLY`).

Closes sbdchd#990

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@netlify

netlify Bot commented Mar 12, 2026

Copy link
Copy Markdown

👷 Deploy request for squawkhq pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit ab1d5ba

@reteps reteps changed the title linter: add file-level override for assume_in_transaction linter: add file-level override for assume_in_transaction Mar 12, 2026
@reteps reteps marked this pull request as ready for review March 12, 2026 15:29
Comment thread crates/squawk_linter/src/rules/transaction_nesting.rs
@reteps reteps marked this pull request as draft April 1, 2026 19:08
@reteps

reteps commented Apr 1, 2026

Copy link
Copy Markdown
Author

Marking as draft till this is revamped given @sbdchd's preferences on directions to go in.

const ENABLE_ASSUME_IN_TRANSACTION: &str = "squawk-enable-assume-in-transaction";

pub fn find_transaction_override(file: &SyntaxNode) -> Option<bool> {
for event in file.preorder_with_tokens() {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also seems like we'd only want to check the first 20 lines rather than have to look at every comment in the file (including ones that are nested inside other nodes)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore is a little different since lint errors can occur ~anywhere

@reteps reteps changed the title linter: add file-level override for assume_in_transaction linter: add file-level override to disable assume_in_transaction May 1, 2026
Only the `squawk-disable-assume-in-transaction` file-level override
remains. Renames `find_transaction_override` to
`has_disable_assume_in_transaction` since it now only signals one
direction.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@reteps reteps requested a review from sbdchd May 1, 2026 19:46
@reteps reteps marked this pull request as ready for review May 1, 2026 19:46

@sbdchd sbdchd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sbdchd sbdchd added the automerge automerge with kodiak label May 3, 2026
@kodiakhq kodiakhq Bot merged commit 8f7af55 into sbdchd:master May 3, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge with kodiak

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File-level disable for assume_in_transaction

2 participants