Skip to content

feat: Show list of ignored gems - #2354

Open
written-fresh wants to merge 1 commit into
Shopify:mainfrom
written-fresh:feat/show-skipped-gems
Open

feat: Show list of ignored gems#2354
written-fresh wants to merge 1 commit into
Shopify:mainfrom
written-fresh:feat/show-skipped-gems

Conversation

@written-fresh

@written-fresh written-fresh commented Jul 20, 2025

Copy link
Copy Markdown
Contributor

Motivation

This PR addresses #1347 which aims to show a message when trying to run tapioca gem on a ignored gem.

Implementation

I have added a skipped gems property to the cli.

Tests

I have now written a test just to confirm something like gem sorbet outputs the warning that it was skipped. More than happy to add more if needed.

No warning

Showing no warning

Warning

image

@written-fresh
written-fresh force-pushed the feat/show-skipped-gems branch from 1ee1e56 to 6645633 Compare August 4, 2026 22:21
@written-fresh
written-fresh marked this pull request as ready for review August 4, 2026 22:24
@written-fresh
written-fresh requested a review from a team as a code owner August 4, 2026 22:24
Comment thread spec/tapioca/cli/gem_spec.rb Outdated
Comment thread lib/tapioca/cli.rb Outdated
Comment thread lib/tapioca/commands/gem_generate.rb Outdated
@written-fresh
written-fresh force-pushed the feat/show-skipped-gems branch from 51a7713 to 28d3674 Compare August 5, 2026 15:22
@written-fresh
written-fresh force-pushed the feat/show-skipped-gems branch from 28d3674 to bcc671b Compare August 5, 2026 15:25

@KaanOzkan KaanOzkan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you, few more refactors. Also there might be some long lines, it'd be good to run rubocop before pushing.

Comment on lines +20 to +22
gem_queue = gems_to_generate(@gem_names)
user_excluded_gems = user_excluded_gem_names(gem_queue)
gem_queue.reject! { |gem| @exclude.include?(gem.name) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
gem_queue = gems_to_generate(@gem_names)
user_excluded_gems = user_excluded_gem_names(gem_queue)
gem_queue.reject! { |gem| @exclude.include?(gem.name) }
user_excluded_gems = @gem_names & @exclude
gem_queue = gems_to_generate(@gem_names)
gem_queue.reject! { |gem| @exclude.include?(gem.name) }

Simpler

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also I think we should move user_excluded_gems assignment right before it's printed. It's nicer I believe.

def gems_to_generate(gem_names)
return @bundle.dependencies if gem_names.empty?

(gem_names - @exclude).each_with_object([]) do |gem_name, gems|

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
gem_names.each_with_object([]) do |gem_name, gems|

Comment on lines +84 to +89
def user_excluded_gem_names(gem_queue)
@exclude.uniq.select do |gem_name|
@bundle.gem(gem_name) &&
(@gem_names.include?(gem_name) || gem_queue.any? { |gem| gem.name == gem_name })
end
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
def user_excluded_gem_names(gem_queue)
@exclude.uniq.select do |gem_name|
@bundle.gem(gem_name) &&
(@gem_names.include?(gem_name) || gem_queue.any? { |gem| gem.name == gem_name })
end
end

@@ -44,6 +46,14 @@
else
say("No operations performed, all RBIs are up-to-date.", [:green, :bold])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be printed at the end, let's move the new prints before this if/else.

say("No operations performed, all RBIs are up-to-date.", [:green, :bold])
end
unless @skipped_gems.empty?
say("\nNote: Tapioca is skipping gem rbi generation for following gems due to the built-in configuration:", [:yellow, :bold])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
say("\nNote: Tapioca is skipping gem rbi generation for following gems due to the built-in configuration:", [:yellow, :bold])
say("\nNote: Tapioca skipped RBI generation for the following gems because they are ignored by default:", [:yellow, :bold])

say(@skipped_gems.join(", "), [:yellow, :bold])
end
unless user_excluded_gems.empty?
say("\nNote: Tapioca is skipping gem rbi generation for following gems due to user configuration:", [:yellow, :bold])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
say("\nNote: Tapioca is skipping gem rbi generation for following gems due to user configuration:", [:yellow, :bold])
say("\nNote: Tapioca skipped RBI generation for the following gems because they were excluded:", [:yellow, :bold])

assert_success_status(result)
end

it "reports explicitly requested ignored gems" do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you also add a test for not reporting excluded gems when we only run gem and gem --all?

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.

2 participants