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

Deprecate `brew cask update`. #1690

Merged
merged 7 commits into from Dec 30, 2016

Conversation

Projects
None yet
4 participants
@reitermarkus
Copy link
Member

reitermarkus commented Dec 16, 2016

  • 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?

In addition to deprecating brew cask update I also added a new option to odeprecated which allows us to specify a date after which a given method will be disabled.


Closes Homebrew/homebrew-cask#26921.

@vitorgalvao

This comment has been minimized.

Copy link
Member

vitorgalvao commented Dec 16, 2016

Also needs to be removed from the man page and brew cask help.

@reitermarkus

This comment has been minimized.

Copy link
Member

reitermarkus commented Dec 17, 2016

Updated the manpage, but can't really remove it from brew cask help easily due to the logic handling this, so I added a warning instead.

@reitermarkus reitermarkus force-pushed the reitermarkus:brew-cask-update branch from 4654a21 to bd64507 Dec 17, 2016

@@ -0,0 +1,17 @@
module Hbc

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Dec 29, 2016

Member

I'd maybe go Library/Homebrew/hbc/compat/cli/update.rb for this.

This comment has been minimized.

@reitermarkus

reitermarkus Dec 30, 2016

Member

I think it's best to keep all of these in one central location.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Dec 30, 2016

Member

Until the Homebrew Cask code is structured and the compatibility layer is a lot closer to the rest of Homebrew I disagree. Compared to the other compat modules this one is included as-is regardless of a --no-compat or similar, isn't included into Homebrew's compat.

This comment has been minimized.

@reitermarkus

reitermarkus Dec 30, 2016

Member

this one is included as-is regardless of a --no-compat

No, it isn't. Everything under compat/hbc is not included when --no-compat is specified.

This comment has been minimized.

@reitermarkus

reitermarkus Dec 30, 2016

Member

Ah, right, sorry, I missed the require "compat/hbc/cli/update" line.

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Dec 30, 2016

Member

Ok, I see. I'm fine with this then 👍

replacement_message = if replacement
"Use #{replacement} instead."
else
"There is no replacement."
end

unless disable_on.nil?
if disable_on > Time.now
will_be_disabled_message = " and will be disabled on #{disable_on.strftime("%B %-d, %Y")}"

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Dec 29, 2016

Member

Let's go for a "29 May 2017" date format.

This comment has been minimized.

@reitermarkus

reitermarkus Dec 30, 2016

Member

Went with ISO 8601.

verb = if disable
"disabled"
else
"deprecated".concat(will_be_disabled_message.to_s)

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Dec 29, 2016

Member

Just interpolate this I think.

This comment has been minimized.

@reitermarkus

@reitermarkus reitermarkus force-pushed the reitermarkus:brew-cask-update branch 2 times, most recently from dd4c671 to c344435 Dec 30, 2016

@reitermarkus reitermarkus force-pushed the reitermarkus:brew-cask-update branch from c344435 to ddaf173 Dec 30, 2016

@reitermarkus

This comment has been minimized.

Copy link
Member

reitermarkus commented Dec 30, 2016

Just noticed that we do in fact have an easy way to hide the update command from help.

@reitermarkus reitermarkus merged commit e2689a6 into Homebrew:master Dec 30, 2016

3 checks passed

codecov/patch 71.42% of diff hit (target 63.01%)
Details
codecov/project 63.01% (+<.01%) compared to 4ca2eaf
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@reitermarkus reitermarkus deleted the reitermarkus:brew-cask-update branch Dec 30, 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.