-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add "Nested Transactions" section #355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Thank you for sharing this article. This article seems to be referring to performance issues caused by nested transactions in PostgreSQL.
Please note that this PR is not about whether you should use nested transactions, but only that if you do, you should not use joinable: false. This is a fairly partial mention, so I'm honestly not sure if it should be added to the style guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes total sense.
I apologize for an unrelated reference above.
Here are some related references:
- a PR with a cop corresponding to this guideline rubocop/rubocop-rails#1236
- nested transactions rspec-rails issue with fruitful discussion rspec/rspec-rails#2598
- rubocop/rubocop-rails#400 recommends using requires_new: true
- rubocop/rubocop-rails#414 a corresponding cop (closed as stale)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to what I said in the corresponding RuboCop PR, I think we could group this into a section about avoiding private APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your suggestion.
However, there may be different reasons why private APIs should not be used, so I'm wondering if it's appropriate to group them all in one section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the section should remain as it is because the focus should be more on the issues that arise with nested transactions rather than the use of private APIs.
@wata727 I’ve been reading your blog, and I was wondering if you could elaborate a bit more on after_commit in this style guide?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking about this PR again now, and I'm beginning to think that the section might not be suitable as a "style guide." I believe it’s worth having RuboCop find the problems caused by joinable: false, but it might be endless to list all such problems in the style guide.
@koic Is there a chance we could add a cop without adding it to the style guide, or do you think problems like this are worth mentioning in the style guide?
See also rubocop/rubocop-rails#1236