Skip to content
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

Improve internal API to make bundleup easier to use in Ruby scripts #214

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

inkstak
Copy link
Contributor

@inkstak inkstak commented Aug 11, 2023

I would like to integrate bundleup into an overall update script and perform actions accordingly to the gems that have been updated.

For example:

updates = Bundleup::CLI.new([]).run

if updates.any? && confirm?("Some gems have been updated, would you like to run tests ?")
  system "bundle exec rspec" 
end

if updates.include?("rubocop") && confirm?("Rubocop has been updated, would you like to run cops ?")
  system "bundle exec rubocop"
end

if updates.any? && confirm?("Would you like to commit changes?")
  system "git commit -m \"Update gems dependencies\" -- Gemfile.lock"
end

If you might consider these changes, I will try to add tests.

@mattbrictson
Copy link
Owner

Interesting idea! Let me think this over. I am not opposed to it, but it seems weird for CLI#run to return a value. A CLI, by nature, takes its inputs and provides its outputs via the command-line. So getting back an object from run doesn't feel right.

I would rather the updates be obtained by an explicit method, rather than a return value.

Something like this:

cli = Bundleup::Cli.new([])
cli.run
updates = cli.updated_gems

It's a small distinction. Would that work for you?

@inkstak
Copy link
Contributor Author

inkstak commented Aug 12, 2023

I understand and by extension it could open the way to a pinned_gems method for those who need it.

@mattbrictson
Copy link
Owner

@inkstak great! If you could make that small refactor and add a test, I'll merge this in. I'm also happy to add a test later, if you get stuck on that part.

@inkstak
Copy link
Contributor Author

inkstak commented Aug 15, 2023

Done ! Some tests added and passing.
Feel free to refactor as you want ;)

@mattbrictson
Copy link
Owner

Nice! Tests look great. Much appreciated! 🙏

@mattbrictson mattbrictson added ✨ Feature Adds a new feature automerge Automatically merge PR once all required checks pass labels Aug 18, 2023
@kodiakhq kodiakhq bot merged commit 94044d2 into mattbrictson:main Aug 18, 2023
6 checks passed
@mattbrictson mattbrictson changed the title Return updated gems Improve internal API to make bundleup easier to use in Ruby scripts Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge PR once all required checks pass ✨ Feature Adds a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants