-
Notifications
You must be signed in to change notification settings - Fork 13
Output number of table created from schema.rb or structure.sql, add polymorphic models count #37
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
602404b
to
8174700
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
==========================================
+ Coverage 82.86% 83.20% +0.33%
==========================================
Files 19 19
Lines 683 750 +67
==========================================
+ Hits 566 624 +58
- Misses 117 126 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0f6be4b
to
baa9321
Compare
baa9321
to
4b93f2c
Compare
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.
@hmdros Thanks for submitting this!
This is outside of the scope of #31:
STI models count: 1 STI classes
And it can be quite complex to implement.
Could you please submit a new issue about STIs?
I think we could merge this PR without the STIs code and then you can work on the STIs code in a separate PR.
Thank you!
lib/rails_stats/stats_calculator.rb
Outdated
@sti = 0 | ||
Dir.glob(File.join(@root_directory, "app", "models", "*.rb")).each do |file| | ||
File.foreach(file) do |line| | ||
if line =~ /class\s+(\w+)\s*<\s*(\w+)/ && !(line =~ /class\s+(\w+)\s*<\s*(ActiveRecord::Base|ApplicationRecord)/) |
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.
@hmdros I think this is too optimistic and it will generate false positives in certain applications.
Many projects use app/models as a place to put models that don't necessarily extend an ActiveRecord::Base|ApplicationRecord descendant.
I see that in your test you have this example:
class Pets < User
end
That's certainly an STI.
But what if someone has this in their app/models directory:
class Pets < User
end
And then:
class User
...
end
That is not an STI and it will be counted as one (or two?)
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 do agree that this can get confusing and the code to check this would need to be extra careful for this. I can try to get this in a different PR then. Removing
Closes #31
CHANGELOG.md
that links to this PR under the "main (unreleased)" heading.Description:
Adds to the output:
I will abide by the code of conduct.