Skip to content

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Sep 12, 2014

@bdunne
Copy link
Member Author

bdunne commented Sep 12, 2014

@Fryguy @jrafanie Please review

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 15b3c02 on brandondunne:ensure_yum_update_raises_on_error into 41a7933 on ManageIQ:master.

@bdunne bdunne force-pushed the ensure_yum_update_raises_on_error branch from 15b3c02 to cb337ea Compare September 12, 2014 03:29
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling cb337ea on brandondunne:ensure_yum_update_raises_on_error into 41a7933 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.

This begs for a comment or something.

Does it make sense to use run and instead to make it obvious that yum in some cases is exiting 0 on errors:

raise ... if (out.exit_status != 0) || (out.exit_status == 0 && out.error.include?("No Match for argument"))

Copy link
Member

Choose a reason for hiding this comment

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

Forget if it's exit_code or exit_status....

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather get yum fixed and have it exit 1 and let run! handle it. I'll open a bug on yum and make a comment with the BZ id.

@jrafanie
Copy link
Member

Good find. Just had a few small nitpicks.

@bdunne bdunne force-pushed the ensure_yum_update_raises_on_error branch from cb337ea to a18b3e6 Compare September 12, 2014 16:57
@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2014

Checked commit bdunne@a18b3e6 with rubocop 0.21.0
2 files checked, 3 offenses detected

spec/yum_spec.rb

@bdunne
Copy link
Member Author

bdunne commented Sep 12, 2014

@jrafanie updated based on your comments

@jrafanie
Copy link
Member

@brandondunne Looks good to me, merge at will when green. :shipit:

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling a18b3e6 on brandondunne:ensure_yum_update_raises_on_error into 41a7933 on ManageIQ:master.

bdunne added a commit that referenced this pull request Sep 12, 2014
…_error

Ensure yum update raises on argument errors even with exit code 0
@bdunne bdunne merged commit ca74609 into ManageIQ:master Sep 12, 2014
@bdunne bdunne deleted the ensure_yum_update_raises_on_error branch September 12, 2014 19:40
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