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

Add Arch Linux support to package plugin #1042

Merged
merged 1 commit into from
Sep 10, 2017
Merged

Add Arch Linux support to package plugin #1042

merged 1 commit into from
Sep 10, 2017

Conversation

keeakita
Copy link
Contributor

@keeakita keeakita commented Aug 6, 2017

Description

This commit adds support for detecting packages on Arch Linux. This is my first time contributing to Ohai or any Chef project, so I'd appreciate any feedback.

Issues Resolved

I wrote this mainly because I wanted to make some decisions in Chef that involved knowing if one package from a set were already installed. Specifically, I wanted a recipe that installs command line vim, but only if graphical vim isn't already installed.

Check List

@keeakita
Copy link
Contributor Author

keeakita commented Aug 6, 2017

Dang, I wasn't aware of chefstyle. Let me make some changes and try again.

@@ -31,6 +31,7 @@
end

collect_data(:linux) do
require "date"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this into the Arch section since it's not needed for the other platforms

@@ -31,6 +31,7 @@
end

collect_data(:linux) do
require "date"
packages Mash.new
if %w{debian}.include? platform_family
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance you can change these platform_family checks into a case statement. At 3 different checks of the same data it would be faster to use a single check in a case statement

@tas50
Copy link
Contributor

tas50 commented Sep 7, 2017

Overall this looks pretty solid. Thanks for putting it together. Just fix those 2 comments and this is good to go.

@keeakita keeakita requested a review from a team September 10, 2017 18:53
Signed-off-by: William Osler <oslerw@fb.com>
@tas50 tas50 merged commit dccaaf9 into chef:master Sep 10, 2017
@tas50
Copy link
Contributor

tas50 commented Sep 10, 2017

Looks good

@chef chef locked and limited conversation to collaborators Feb 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants