Include seeds in Rubocop (plus fixes to seeds to satisfy Rubocop)#432
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…uding everything in db
There was a problem hiding this comment.
Pull request overview
This PR enables Rubocop analysis for seed files while explicitly excluding only migrations and schema.rb. The change includes fixes to existing seed files to satisfy Rubocop requirements.
Changes:
- Modified
.rubocop.ymlto evaluate seed files while excluding only migrations and schema.rb - Refactored
db/seeds/users.rbto address code quality issues and improve output formatting - Updated
db/seeds/posts.rbto fix Rubocop violations and improve code style
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .rubocop.yml | Changed exclusion pattern from all db files to only migrations and schema.rb |
| db/seeds.rb | Updated file path syntax to use consistent string format |
| db/seeds/users.rb | Refactored user seeding logic with improved methods and Rubocop compliance |
| db/seeds/posts.rb | Fixed various Rubocop violations including unused variables and code style issues |
| { | ||
| published_at: Faker::Time.between(from: 4.months.ago, to: 1.week.ago), | ||
| status: 2, # published | ||
| published_at: Faker::Time.between(from: 4.months.ago, to: 1.week.ago), status: 2, # published |
There was a problem hiding this comment.
The inline comment placement after status: 2 makes the code harder to read. Consider moving the comment to a separate line above the hash or removing it since the context already makes it clear this is the published status.
| published_at: Faker::Time.between(from: 4.months.ago, to: 1.week.ago), status: 2, # published | |
| published_at: Faker::Time.between(from: 4.months.ago, to: 1.week.ago), | |
| # published | |
| status: 2, |
There was a problem hiding this comment.
Good bot, I generally agree - but choice was to disable the 10 line length rule, or refactor which didn't seem too valuable, so I thought this was a happy enough compromise.
If there's any humans feeling strongly let me know.
| @@ -52,25 +50,24 @@ def post_content_enhanced | |||
| content += Faker::Lorem.paragraph(sentence_count: 8, supplemental: true, random_sentences_to_add: 4) | |||
| content += "<br /><br />" | |||
| end | |||
There was a problem hiding this comment.
The post_content_enhanced method should explicitly return content at the end for clarity. While Ruby returns the last evaluated expression, an explicit return statement improves readability for methods that build up a result.
| end | |
| end | |
| return content |
There was a problem hiding this comment.
Good bot, I generally agree - but choice was to disable the 10 line length rule, or refactor which didn't seem too valuable, so I thought this was a happy enough compromise.
If there's any humans feeling strongly let me know.
2 participants
Review after #431 is merged, as this branch branches off that branch.
The current state is that any file in the
dbdirectory or its subdirectories is not evaluated by Rubocop, but this makes it tedious to apply code formatting and check code quality as we add more seeding code (eg for Campaigns)This change makes it more explicit where we do not care to use Rubocop (db migrations, schema.rb) so that Rubocop will evaluate everything else.
This change also includes the fixes to satisfy Rubocop.
This screenshot shows that the seeding works correctly after the changes.