Skip to content

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Aug 16, 2013

No description provided.

@bdunne
Copy link
Member Author

bdunne commented Aug 16, 2013

@Fryguy @jrafanie Ignore previous comment, fixed the test, ready for review.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling e40de89 on brandondunne:add_exception_for_invalid_credentials into 428621f on ManageIQ:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling eb0d03f on brandondunne:add_exception_for_invalid_credentials into 428621f on ManageIQ:master.

Copy link
Member

Choose a reason for hiding this comment

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

@brandondunne What is the point of an empty rescue/raise? Ruby does the same behavior by default.

@bdunne
Copy link
Member Author

bdunne commented Aug 16, 2013

@chessbyte I didn't realize that, removed the unneeded rescue/raise.

@coveralls
Copy link

Coverage Status

Coverage decreased (-24.94%) when pulling 93c0064 on brandondunne:add_exception_for_invalid_credentials into 428621f on ManageIQ:master.

Copy link
Member

Choose a reason for hiding this comment

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

I think these exception should live under the LinuxAdmin namespace. Not sure if that should happen in this PR or not.

@bdunne
Copy link
Member Author

bdunne commented Aug 16, 2013

@Fryguy Modified as discussed

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f239781 on brandondunne:add_exception_for_invalid_credentials into 428621f on ManageIQ:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f239781 on brandondunne:add_exception_for_invalid_credentials into 428621f on ManageIQ:master.

Fryguy added a commit that referenced this pull request Aug 16, 2013
…edentials

Add exception for invalid credentials
@Fryguy Fryguy merged commit 6961916 into ManageIQ:master Aug 16, 2013
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.

4 participants