Skip to content
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

Disable unavailable extensions (bnc#883148) #113

Merged
merged 3 commits into from Jun 23, 2014

Conversation

lslezak
Copy link
Member

@lslezak lslezak commented Jun 23, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling a4d24a8 on disable_unavailable_extensions into 4fe81a5 on master.

@@ -65,6 +65,7 @@ def create_addon_with_deps(root)
# delegate methods to underlaying suse connect object
def_delegators :@pure_addon,
:arch,
:available,
Copy link
Member

Choose a reason for hiding this comment

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

I see there two checks for false. It have impression that nil is in fact available. I found quite confusing to see that available = nil in fact is available. I recommend to hide such logic into addon class like removing this delegator and instead have

def available?
  # nil mean, that server do not report about availability, so we expect that all is available
  !(@pure_addon.available == false)
end

and then later use intuitive things like unless available?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, good point...

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling ce711f7 on disable_unavailable_extensions into 4fe81a5 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling c3640f5 on disable_unavailable_extensions into 4fe81a5 on master.

expect(addon.selectable?).to be_false
end

it "returns true when the addon is available" do
Copy link
Member

Choose a reason for hiding this comment

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

I miss case with it "returns true when addon availability is not set" do to cover common case.

@jreidinger
Copy link
Member

LGTM

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling fbf3ac6 on disable_unavailable_extensions into 4fe81a5 on master.

lslezak added a commit that referenced this pull request Jun 23, 2014
Disable unavailable extensions (bnc#883148)
@lslezak lslezak merged commit ceddd6a into master Jun 23, 2014
@lslezak lslezak deleted the disable_unavailable_extensions branch June 23, 2014 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants