Skip to content

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Feb 20, 2015

No description provided.

@bdunne
Copy link
Member Author

bdunne commented Feb 20, 2015

@Fryguy Please review

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.31% when pulling 8d3f3d5 on brandondunne:loosen_dependency_on_inifile into 490add8 on ManageIQ:master.

@miq-bot
Copy link
Member

miq-bot commented Feb 20, 2015

Checked commit bdunne@8d3f3d5 with rubocop 0.27.1
0 files checked, 0 offenses detected
Everything looks good. 👍

@Fryguy
Copy link
Member

Fryguy commented Feb 20, 2015

Looks good to me. Note that @movitto recently had to use iniparse (as opposed to inifile) for systemd stuff because it supported multiple keys so we may want to switch to that to stop a shared dependency

@bdunne
Copy link
Member Author

bdunne commented Feb 20, 2015

From IRC:

<bdunne> Hey, when you get a chance, let me know if you have any concerns with this: https://github.com/ManageIQ/linux_admin/pull/113
<mmorsi> hey
<mmorsi> looks fine to me
<mmorsi> didn't test it tho
<bdunne> all the specs pass
<mmorsi> sure
<mmorsi> we can figure out the inifile / iniparse stuff  in hte future as the systemd progresses
<mmorsi> until then no worries
<bdunne> cool
<bdunne> thanks
<mmorsi> np

bdunne added a commit that referenced this pull request Feb 20, 2015
@bdunne bdunne merged commit e5cecaa into ManageIQ:master Feb 20, 2015
@bdunne bdunne deleted the loosen_dependency_on_inifile branch February 20, 2015 18:13
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