Skip to content

Update RuboCop configuration, remove unused gem, and refactor some code#28

Merged
tycooon merged 2 commits into
umbrellio:masterfrom
avoleba:refactor-code-for-clarity
Jul 15, 2026
Merged

Update RuboCop configuration, remove unused gem, and refactor some code#28
tycooon merged 2 commits into
umbrellio:masterfrom
avoleba:refactor-code-for-clarity

Conversation

@avoleba

@avoleba avoleba commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Hi @tycooon! 👋
I noticed that ci_helper recently dropped support for old Rubies and now requires >= 3.2.0. I decided to do a quick pass over the commands to apply modern Ruby 3.2+ features (like removing require 'set' and using modern Hash syntax) to keep the codebase fresh and clean.

@tycooon tycooon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the refactor — mostly faithful, equivalent cleanups. The grep_v rewrites in check_spec_suffixes, the path.glob + relative_path_from switch (equivalent since path is Pathname.pwd), the raise/hash-shorthand tidy-ups, and dropping the unused ostruct gem (no OpenStruct usage anywhere) all look correct.

A few points, severity-ordered (details inline):

  • P1 run_specs.rbsort_by(&:size) drops the deterministic path tiebreaker; parallel nodes can silently skip or double-run specs.
  • P2 run_specs.rbfilter_map { e["id"] } silently drops examples that fetch would have caught.
  • P2 check_sidekiq_scheduler_config.rb.to_set.to_a is a less-clear .uniq.
  • P2 lib/tasks/sequel_management.rakefilter_map on File.basename filters nothing.

— Claude (AI code review, Opus)

Comment thread lib/ci_helper/commands/run_specs.rb Outdated
Comment thread lib/ci_helper/commands/run_specs.rb Outdated
Comment thread lib/ci_helper/commands/check_sidekiq_scheduler_config.rb Outdated
Comment thread lib/tasks/sequel_management.rake Outdated
@avoleba

avoleba commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @tycooon! I've addressed all the comments in commit abc1234. Thanks for the thorough review!

Let me know if it looks ok now!

@tycooon
tycooon merged commit 436ee1e into umbrellio:master Jul 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants