Skip to content

Adds BundleReport CLI class to an entry point from executable file #154

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

Merged
merged 13 commits into from
May 6, 2025

Conversation

hmdros
Copy link

@hmdros hmdros commented May 5, 2025

Description

Following issue: #153

Motivation and Context

Exposing a single entry point for the bundle_report command to make it easier to invoke programmatically.

Remove the logic from exe/bundle_report, and move it out to lib/next_rails/bundle_report/cli.rb so we can call: NextRails::BundleReport::CLI.new(ARGV).generate from another service, and we would still have the same output as calling the bundle_report from exe.

How Has This Been Tested?

Moving the code from exe/bundle_report to lib/next_rails/bundle_report/cli.rb should keep the same behavior.

I will abide by the code of conduct

Copy link
Member

@JuanVqz JuanVqz left a comment

Choose a reason for hiding this comment

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

Could you please add a test? Is it complicated?

@@ -1,8 +1,12 @@
# frozen_string_literal: true

require 'optparse'
require 'next_rails'
require 'next_rails/bundle_report'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
require 'next_rails/bundle_report'
require 'next_rails/bundle_report'

# Print a report on our Gemfile
# Why not just use `bundle outdated`? It doesn't give us the information we care about (and it fails).
#
at_exit do
Copy link
Author

Choose a reason for hiding this comment

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

Was there an specific reason for using this at_exit block?
I removed it from def run in the CLI because it was making tests execution fail for it

Copy link
Member

@JuanVqz JuanVqz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the contribution @hmdros :shipit:

@JuanVqz JuanVqz merged commit 3d35c23 into main May 6, 2025
8 checks passed
@JuanVqz JuanVqz deleted the bundle_report_cli branch May 6, 2025 20:28
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.

5 participants