Repository navigation
Show versions of dependencies in dry-run - #24210
Conversation
f638049 to
b846a4f
Compare
53f5e31 to
6fc4def
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1c202911-7b21-4ec3-b3bd-b6c2fe91b281
📒 Files selected for processing (11)
Library/Homebrew/cask/upgrade.rbLibrary/Homebrew/cmd/upgrade.rbLibrary/Homebrew/install.rbLibrary/Homebrew/test/cask/upgrade_spec.rbLibrary/Homebrew/test/cmd/install_spec.rbLibrary/Homebrew/test/cmd/upgrade_spec.rbLibrary/Homebrew/test/install_spec.rbLibrary/Homebrew/test/upgrade_spec.rbLibrary/Homebrew/test/utils/formatter_spec.rbLibrary/Homebrew/upgrade.rbLibrary/Homebrew/utils/formatter.rb
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines (1)
Generated by CodeRabbit — auto-discovered
📓 Path-based instructions (2)
Test command behavior and its observable effects, including stdout, stderr, exit status, and state changes as relevant.
📄 CodeRabbit inference engine (Generated by CodeRabbit)
Files:
Library/Homebrew/test/cmd/install_spec.rbLibrary/Homebrew/test/cmd/upgrade_spec.rb
Implement CLI commands as `Homebrew::Cmd` classes inheriting from `AbstractCommand`; declare help text and argument metadata in `cmd_args`, and put command behavior in `run`.
📄 CodeRabbit inference engine (Generated by CodeRabbit)
Files:
Library/Homebrew/cmd/upgrade.rb
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Sorry, not a fan. If the goal is to be more consistent, let's discuss deparenthesising some other cases.
6fc4def to
fa04d7e
Compare
fa04d7e to
8f07cd9
Compare
|
@MikeMcQuaid Makes sense, removed the parantheses but kept the other changes. |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks, looks nice! Maybe just do the TODO now?
Sure, I don't mind! But to do it properly I would like to touch every place on the code that passes pkg strings and parses like this. That might be a bigger change. |
8f07cd9 to
95efba9
Compare
|
@MikeMcQuaid Done! |
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Much nicer! One more small tweak then good to go.
8ff8245 to
0668c7a
Compare
- `brew install --dry-run` listed dependencies by name only; show the version to install or `old -> new` as `brew upgrade --dry-run` does. - Pass `VersionChange` values to `Formatter.version_changes` instead of formatting and re-parsing strings. - Align download sizes in their own column in upgrade summaries.
0668c7a to
70b8a8f
Compare
brew install --dry-runlists the dependencies it would install or upgrade by name only, so you cannot see which versions would be installed.brew upgrade --dry-runalready shows them.This shows
name version(orname old -> newfor upgrades) for those dependencies, aligned in columns. The aligned upgrade summaries also now put download sizes in their own column instead of leaving them glued to the end of the version. The shared formatting moves toFormatter.version_changes, which now takes a newVersionChangevalue (name, old and new version, size) instead of formatted strings, so nothing has to be parsed back.Before:
After:
Upgrade summaries with sizes, before:
After:
brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Claude Code (Claude Sonnet 5.5) helped write the change and its specs. I reviewed the output. Typecheck, style on
formatter.rband the install, upgrade, formatter,cask/upgrade,cmd/installandcmd/upgradespecs were run locally.