Fixes for internal messaging #14571

EricFromCanada · 2023-02-09T17:20:05Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

Fixes for a wide variety of style and formatting nits in brew's messaging and manpage. This companion messaging-fixes branch has them broken down by category.

BrewTestBot · 2023-02-09T17:20:32Z

Review period will end on 2023-02-10 at 17:20:05 UTC.

carlocab · 2023-02-10T03:05:29Z

Library/Homebrew/brew.sh

+to either the standard prefix for your platform or to a non-standard prefix that
+is not in the Homebrew temporary directory.


I think we have an environment variable for the default prefix somewhere.

carlocab · 2023-02-10T03:13:14Z

Library/Homebrew/cmd/install.rb

@@ -30,7 +30,7 @@ def install_args
        Unless `HOMEBREW_NO_INSTALL_CLEANUP` is set, `brew cleanup` will then be run for
        the installed formulae or, every 30 days, for all formulae.

-        Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install <formula>` will upgrade <formula> if it
+        Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install` <formula> will upgrade <formula> if it


If you don't want to keep <formula> inside the backticks:

Suggested change

Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install` <formula> will upgrade <formula> if it

Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install foo`, where `foo` is the name of a formula, will upgrade `foo` if it

I prefer the <formula> usage but agree having it in backticks might be nicer

Anything in angle brackets becomes monospaced italic in the Markdown output, but it needs to be outside a backticked sequence for it to output properly, i.e.

`brew install` *`formula`* # correct `brew install *`formula`*` # not correct

This is because Markdown allows a monospaced sequence to be italicized in its entirety using delimiters outside the backticks, but not within.

carlocab · 2023-02-10T03:13:56Z

Library/Homebrew/cmd/update.sh

-        # HOMEBREW_CURL is set by brew.sh (and isn't mispelt here)
+        # HOMEBREW_CURL is set by brew.sh (and isn't misspelt here)


carlocab · 2023-02-10T03:15:07Z

Library/Homebrew/dev-cmd/dispatch-build-bottle.rb

@@ -21,8 +21,8 @@ def dispatch_build_bottle_args
             description: "Build timeout (in minutes, default: 60)."
      flag   "--issue=",
             description: "If specified, post a comment to this issue number if the job fails."
-      comma_array "--macos=",
-                  description: "Version(s) of macOS the bottle should be built for."
+      comma_array "--macos",


The other args end with = here though. Does this change the usage of the command?

The = only has meaning for flag, indicating to the parser that an argument is required. It has no meaning for switch, which never takes arguments, nor for comma_array, which always requires them (and eats any = that follows). In retrospect it may have been better to use a more obvious convention, e.g. flag_opt / flag_req or similar.

carlocab · 2023-02-10T03:16:58Z

Library/Homebrew/dev-cmd/vendor-gems.rb

@@ -16,7 +16,7 @@ def vendor_gems_args
      EOS

      comma_array "--update",
-                  description: "Update all vendored Gems to the latest version."
+                  description: "Update the specified list of vendored gems to the latest version."


What happens if I don't specify any arguments to --update?

Error: missing argument: --update

carlocab · 2023-02-10T03:18:41Z

Library/Homebrew/test/cmd/--prefix_spec.rb

@@ -27,7 +27,7 @@
      .and be_a_failure
  end

-  it "prints a warning with `--installed` if the given Formula is not installed", :integration_test do
+  it "prints a warning when using `--installed` if the given Formula is not installed", :integration_test do


Suggested change

it "prints a warning when using `--installed` if the given Formula is not installed", :integration_test do

it "prints a warning when `--installed` is used and the given Formula is not installed", :integration_test do

carlocab · 2023-02-10T03:19:56Z

This companion messaging-fixes branch has them broken down by category.

Might be nicer to use that branch here so that we can see the commits in your PR.

MikeMcQuaid · 2023-02-10T08:53:24Z

Library/Homebrew/env_config.rb

@@ -98,15 +98,15 @@ module EnvConfig
                     "`~/.profile`, `~/.bash_profile`, or `~/.zshenv`:" \
                     '\n\n    `export HOMEBREW_CASK_OPTS="--appdir=~/Applications --fontdir=/Library/Fonts"`',
      },
+      HOMEBREW_CLEANUP_MAX_AGE_DAYS:             {


A lint to ensure these are sorted some time would be nice. Either a help wanted issue or PR would be great

BrewTestBot · 2023-02-10T18:02:14Z

Review period ended.

BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 9, 2023

EricFromCanada force-pushed the messaging-fixes-combined branch from 45cdb3b to b2d6915 Compare February 9, 2023 17:35

carlocab reviewed Feb 10, 2023

View reviewed changes

MikeMcQuaid reviewed Feb 10, 2023

View reviewed changes

MikeMcQuaid approved these changes Feb 10, 2023

View reviewed changes

BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 10, 2023

BrewTestBot approved these changes Feb 10, 2023

View reviewed changes

internal messaging fixes

dfc9906

EricFromCanada force-pushed the messaging-fixes-combined branch from b2d6915 to dfc9906 Compare February 11, 2023 04:17

EricFromCanada merged commit 5cfb179 into Homebrew:master Feb 11, 2023

EricFromCanada deleted the messaging-fixes-combined branch February 11, 2023 04:57

github-actions bot added the outdated PR was locked due to age label Mar 14, 2023

github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for internal messaging #14571

Fixes for internal messaging #14571

EricFromCanada commented Feb 9, 2023

BrewTestBot commented Feb 9, 2023

carlocab Feb 10, 2023

carlocab Feb 10, 2023

MikeMcQuaid Feb 10, 2023

EricFromCanada Feb 11, 2023

carlocab Feb 10, 2023

carlocab Feb 10, 2023

EricFromCanada Feb 11, 2023

carlocab Feb 10, 2023

EricFromCanada Feb 11, 2023

carlocab Feb 10, 2023

carlocab commented Feb 10, 2023

MikeMcQuaid Feb 10, 2023

BrewTestBot commented Feb 10, 2023

		to either the standard prefix for your platform or to a non-standard prefix that
		is not in the Homebrew temporary directory.

	Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install` <formula> will upgrade <formula> if it
	Unless `HOMEBREW_NO_INSTALL_UPGRADE` is set, `brew install foo`, where `foo` is the name of a formula, will upgrade `foo` if it

		# HOMEBREW_CURL is set by brew.sh (and isn't mispelt here)
		# HOMEBREW_CURL is set by brew.sh (and isn't misspelt here)

	it "prints a warning when using `--installed` if the given Formula is not installed", :integration_test do
	it "prints a warning when `--installed` is used and the given Formula is not installed", :integration_test do

Fixes for internal messaging #14571

Fixes for internal messaging #14571

Conversation

EricFromCanada commented Feb 9, 2023

BrewTestBot commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlocab commented Feb 10, 2023

Choose a reason for hiding this comment

BrewTestBot commented Feb 10, 2023