This repository has been archived by the owner. It is now read-only.

subversion: Override 'fatal?' on universal requirements #13248

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

jeffstyr commented Jul 6, 2012

The universal requirements in the subversion formula currently only issue warnings if unsatisfied, so builds are allowed to proceed and then ultimately fail, because the 'fatal?' method on the Requirements class defaults to false. Overriding this to return true allows failing universal requirements to fail the install.

@jeffstyr jeffstyr subversion: Override 'fatal?' on universal requirements
The universal requirements in the subversion formula currently only issue warnings if unsatisfied, so builds are allowed to proceed and then ultimately fail, because the 'fatal?' method on the Requirements class defaults to false. Overriding this to return true allows failing universal requirements to fail the install.

Also refactor universal requirement subclasses to remove redundant code.
6f178c8
Contributor

adamv commented Jul 6, 2012

I'm +1 on making these fatal, but -1 on the refactor; we want to ultimately refactor this a different way.

Contributor

jeffstyr commented Jul 6, 2012

Okay the refactor was there because I have a fix for the serf dependency too (it's currently just silently dropped if serf is already installed non-universal) and it makes for a lot of code duplication. I can remove the refactor but since it's local to this formula it won't make a different refactor in the future harder, but will make things simpler/clearer in the mean time. Let me know.

Contributor

jeffstyr commented Jul 6, 2012

The serf dependency fix I'm talking about is here: jeffstyr/homebrew@9c8c62d

Contributor

adamv commented Jul 6, 2012

Mea culpa. I didn't think to add a universal check for this dep when I pulled it.

Contributor

adamv commented Jul 7, 2012

Addressed in 19c537f

@adamv adamv closed this Jul 7, 2012

Contributor

jeffstyr commented Jul 7, 2012

Wow, you really do like code duplication. :)

Contributor

jeffstyr commented Jul 7, 2012

Side-issue (and probably too much of a hassle to change now): It's peculiar that Requirements default to being...not required. (I can understand a non-required Dependency, but not a non-required Requirement.)

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

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