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

java_requirement: support prompting users to install legacy Java casks #3352

Merged
merged 1 commit into from Oct 26, 2017

Conversation

Projects
None yet
3 participants
@JCount
Member

JCount commented Oct 22, 2017

  • 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 adds support for prompting the user to install an older, "legacy" java cask when required.

Instead of always attempting to satisfy the requirement with the default java cask, currently Java 9, this prompts the user to install a specific java cask version as needed.

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Oct 22, 2017

Member

Thanks for this, @JCount!

Member

MikeMcQuaid commented Oct 22, 2017

Thanks for this, @JCount!

@JCount

This comment has been minimized.

Show comment
Hide comment
@JCount

JCount Oct 22, 2017

Member

Note that the changes I made are very rough around the edges, and will probably need a fair amount of polishing through code review. 🙂

Member

JCount commented Oct 22, 2017

Note that the changes I made are very rough around the edges, and will probably need a fair amount of polishing through code review. 🙂

@MikeMcQuaid

Code looks good. When you've tested it locally for the various branches and are happy with it 👍 to 🚢. If you're feeling generous: a test would be nice, too.

@@ -1,13 +1,30 @@
class JavaRequirement < Requirement
cask "java"

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Oct 26, 2017

Member

The @cask doesn't seem to be set any more, though. Does the cask message still get output as expected now? If so: 🆒 and ignore me.

@MikeMcQuaid

MikeMcQuaid Oct 26, 2017

Member

The @cask doesn't seem to be set any more, though. Does the cask message still get output as expected now? If so: 🆒 and ignore me.

This comment has been minimized.

@JCount

JCount Oct 26, 2017

Member

The base Requirement class has:

class Requirement
  include Dependable

  attr_reader :tags, :name, :cask, :download, :default_formula

  def initialize(tags = [])
    @default_formula = self.class.default_formula
    @cask ||= self.class.cask
    @download ||= self.class.download
    @formula = nil
    tags.each do |tag|
      next unless tag.is_a? Hash
      @cask ||= tag[:cask]
      @download ||= tag[:download]
    end
    @tags = tags
    @tags << :build if self.class.build
    @name ||= infer_name
  end
@JCount

JCount Oct 26, 2017

Member

The base Requirement class has:

class Requirement
  include Dependable

  attr_reader :tags, :name, :cask, :download, :default_formula

  def initialize(tags = [])
    @default_formula = self.class.default_formula
    @cask ||= self.class.cask
    @download ||= self.class.download
    @formula = nil
    tags.each do |tag|
      next unless tag.is_a? Hash
      @cask ||= tag[:cask]
      @download ||= tag[:download]
    end
    @tags = tags
    @tags << :build if self.class.build
    @name ||= infer_name
  end

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Oct 26, 2017

Member

🆒. If it works and you're happy with it: 🚢

@MikeMcQuaid

MikeMcQuaid Oct 26, 2017

Member

🆒. If it works and you're happy with it: 🚢

java_requirement: support prompting users to install legacy Java casks
This enhances the Java requirement to support prompting the user
to install the correct legacy Java version via Cask for formulae
that don't yet work with the latest version of Java. Previously,
even if the formula had a strict requirement that a specific,
older version of Java be used, the messaging would tell the user to
`brew cask install java` (i.e. to install the latest version of Java),
which wouldn't actually satisfy the requirement.

@JCount JCount merged commit badbb00 into Homebrew:master Oct 26, 2017

3 checks passed

codecov/patch 80% of diff hit (target 68.51%)
Details
codecov/project 68.6% (+0.08%) compared to 4d02b96
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@JCount JCount deleted the JCount:legacy-java-cask-requirement-support branch Oct 26, 2017

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.