Skip to content

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jan 24, 2014

No description provided.

@bdunne
Copy link
Member Author

bdunne commented Jan 24, 2014

@Fryguy Please review

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 797a5ca on brandondunne:add_rpm_import_key into 385ee8c 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.

Why not use run! ? I would expect that if I send in a bad file or the wrong path it would blow up.

@bdunne
Copy link
Member Author

bdunne commented Jan 24, 2014

@Fryguy updated

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e7dea65 on brandondunne:add_rpm_import_key into 385ee8c 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.

Are these two comments regarding returning true still right?

@jrafanie
Copy link
Member

Looks good other than my comment above 👍

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 22aa2c1 on brandondunne:add_rpm_import_key into 385ee8c on ManageIQ:master.

Fryguy added a commit that referenced this pull request Jan 25, 2014
@Fryguy Fryguy merged commit faae78d into ManageIQ:master Jan 25, 2014
@bdunne bdunne deleted the add_rpm_import_key branch January 27, 2014 14: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