Skip to content

Fix disable and deprecate reasons #8530

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

Merged
merged 3 commits into from
Aug 31, 2020
Merged

Fix disable and deprecate reasons #8530

merged 3 commits into from
Aug 31, 2020

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Aug 29, 2020

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

Follow up to #8512

This PR does several things:

  1. Fixes an issue where brew style would interpret because: as date:
  2. Extends date checking to disable! in addition to deprecate
  3. Check the because: string to make sure that:
    1. The string does not start with it
    2. The string does not end in ., ! or ?
  4. Changes the message when installing a deprecated or disabled formula to add it before the reason
  5. Allows the use of symbols for common reasons in disable! and deprecate!

Now, both of the following are supported:

disable! because: :no_license
disable! because: "has been replaced by foo"

Here is the list of symbols and their corresponding strings that I added in this PR (of course, these can be changed as needed):

  • :does_not_build: does not build
  • :no_license: has no license
  • :repo_archived: has an archived upstream repository
  • :repo_removed: has a removed upstream repository
  • :unmaintained: is not maintained upstream
  • :unsupported: is not supported upstream
  • :deprecated_upstream: is deprecated upstream
  • :versioned_formula: is a versioned formula

Sorry, something went wrong.

@Rylan12
Copy link
Member Author

Rylan12 commented Aug 29, 2020

This PR is blocked by #8533. Once #8533 has been merged, this can be rebased on to master (and will need to drop f2b02487dc705762899468700094e4a276b23015).

This will still need to wait on Homebrew/homebrew-core#60321 and Homebrew/homebrew-core#60336 to be merged into homebrew-core and linuxbrew-core before this will be good to go, though.

@MikeMcQuaid MikeMcQuaid merged commit 2a4c9f6 into Homebrew:master Aug 31, 2020
@MikeMcQuaid
Copy link
Member

Thanks again @Rylan12!

@Rylan12
Copy link
Member Author

Rylan12 commented Aug 31, 2020

Oops, you're right, @SeekingMeaning. Good catch! (could have sworn I tested it...). I'll get a fix up soon

Edit: fix in #8549

@Rylan12 Rylan12 deleted the fix-disable-and-deprecate-reasons branch August 31, 2020 17:03
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 14, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants