Add spaces between functions and triggers in schema.rb#195
Merged
teoljungberg merged 7 commits intoteoljungberg:masterfrom Jan 31, 2026
Merged
Add spaces between functions and triggers in schema.rb#195teoljungberg merged 7 commits intoteoljungberg:masterfrom
teoljungberg merged 7 commits intoteoljungberg:masterfrom
Conversation
I've been dealing with the changes to schema_dumper in Rails 8.1 and Senic 1.9 and found that it's most common for their methods not to emit a newline after their last operation. Hence I think it's safe to assume that functions() and triggers() should emit a newline before their loops and within their loops, up to the last iteration. And, not having these functions emit a final newline holds with the pattern used by the Rails and Scenic schema_dumpers. I've tested this in my own apps with a patch on this gem and the spacing is correct with the new Rails 8.1 schema changes (and with Scenic emitting `create_view` statements before the `create_function` and `create_trigger` statements).
Contributor
Author
|
@teoljungberg, any opposition? |
Contributor
Author
|
@teoljungberg, just nudging. |
Owner
|
I don't quote understand this patch, what it aims to solve, without a test or similar showcasing why. |
teoljungberg
requested changes
Jan 19, 2026
Owner
teoljungberg
left a comment
teoljungberg
left a comment
There was a problem hiding this comment.
Please come with tests, a diff file we can review, but something to showcase the behavior
Contributor
Author
Thanks for the review. I'll add a test to show where spaces are being emitted. |
Contributor
Author
|
@teoljungberg, made those changes, ready for another review. |
Contributor
Author
|
@teoljungberg nudge |
Owner
|
Thank you @chiperific ! |
Owner
|
I added an extra test to document when there is nothing to dump as b0b9c3f |
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.
I've been dealing with the changes to schema_dumper in Rails 8.1 and Senic 1.9 and found that it's most common for their methods not to emit a newline after their last operation.
Hence I think it's safe to assume that functions() and triggers() should emit a newline before their loops and within their loops, up to the last iteration.
And, not having these functions emit a final newline holds with the pattern used by the Rails and Scenic schema_dumpers.
I've tested this in my own apps with a patch on this gem and the spacing is correct with the new Rails 8.1 schema changes (and with Scenic emitting
create_viewstatements before thecreate_functionandcreate_triggerstatements).