Skip to content

Conversation

@narendasan
Copy link
Contributor

  • 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 tests with your changes locally?

This PR should address Homebrew/homebrew-cask#25127

The message printed when brew cask errors out has been changed from a generic command to pointing at the brew cask documentation on github so that instructions can be changed outside of brew.

There are 2 questions that I have (that should not effect this PR):

Copy link
Contributor

Choose a reason for hiding this comment

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

This empty line can go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the newline the error message and the instructions get concatenated together. Would it be better to change line 155 in cli.rb to add a newline to the end of the error message?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that sounds like more of an issue with the call site than with the message itself.

@jawshooah jawshooah force-pushed the cask_update_error_msgs branch from 96eee77 to 87c5e6b Compare October 17, 2016 20:51
@jawshooah jawshooah force-pushed the cask_update_error_msgs branch from 87c5e6b to 5071271 Compare October 17, 2016 20:52
Copy link
Member

@reitermarkus reitermarkus Oct 17, 2016

Choose a reason for hiding this comment

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

I'd use “Follow” instead of “Try following”.

@jawshooah
Copy link
Contributor

@narendasan I cleaned up your branch to try and fix the CI errors, but it looks like it didn't work. You'll need to git pull --rebase to bring your branch up to date and address @reitermarkus's comment.

@vitorgalvao vitorgalvao added the cask Homebrew Cask label Oct 17, 2016
@narendasan narendasan force-pushed the cask_update_error_msgs branch from e94ce66 to c7ac466 Compare October 18, 2016 03:27
@narendasan
Copy link
Contributor Author

I have the required tests working but I am not sure what I need to modify for the codecov checks to pass

@jawshooah jawshooah force-pushed the cask_update_error_msgs branch from e758702 to 5f29297 Compare October 18, 2016 13:53
@jawshooah jawshooah merged commit b6a0fdd into Homebrew:master Oct 18, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cask Homebrew Cask

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants