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.

Sign up for GitHub

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

Jump to bottom

Added disk_type field to disk fact #2145

Merged
merged 1 commit into from
Oct 21, 2020
Merged

Added disk_type field to disk fact #2145

merged 1 commit into from
Oct 21, 2020

Conversation

kozl
Copy link
Contributor

@kozl kozl commented Oct 16, 2020

No description provided.

@kozl kozl requested review from a team October 16, 2020 00:07
@puppetlabs-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@kozl
Copy link
Contributor Author

kozl commented Oct 16, 2020

I didn't quite understand how to label my PR, it has feature and community labels. Is there any chance this being merged? I'll fix the tests!

@puppetcla
Copy link

CLA signed by all contributors.

@oanatmaria oanatmaria added community enhancement New feature or enhancement labels Oct 16, 2020
@BogdanIrimie
Copy link
Contributor

BogdanIrimie commented Oct 16, 2020 •
edited
Loading

I didn't quite understand how to label my PR, it has feature and community labels. Is there any chance this being merged? I'll fix the tests!

Hi @kozl

Sure, we welcome contributions from the community. Please fix the test and add unit test.

In order to see this fact in Facter's results you will also have to create a fact that calls this resolver. Have a look at https://github.com/puppetlabs/facter/blob/main/lib/facter/facts/linux/disks.rb you fact should be simpler I think, similar to https://github.com/puppetlabs/facter/blob/main/lib/facter/facts/linux/facterversion.rb

@kozl kozl force-pushed the patch-1 branch 2 times, most recently from 0f2fae4 to e5b2a50 Compare October 20, 2020 22:45
@kozl
Copy link
Contributor Author

kozl commented Oct 20, 2020

Ready to merge : )
I've extended existing fact 'disks', so I don't need to create another fact. Am i right?

@kozl
(FACT-2844) Added disk_type field to disk fact
530c928 
'disk' fact was extended with :type field, which indicates disk type:
'ssd' or 'hdd' depending on the value of queue/rotational file in sysfs
@BogdanIrimie
Copy link
Contributor

Jenkins please test this on all

@BogdanIrimie
Copy link
Contributor

BogdanIrimie commented Oct 21, 2020 •
edited
Loading

Ready to merge : )
I've extended existing fact 'disks', so I don't need to create another fact. Am i right?

Great! Looks good, you are right, there is no need to add a new fact. I have added a Jira ticket and prepended it's number to your commit message. Feel free to add more details to the Jira ticket if you want.

@BogdanIrimie BogdanIrimie merged commit 91231bf into puppetlabs:main Oct 21, 2020
@puppetcla
Copy link

CLA signed by all contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BogdanIrimie BogdanIrimie BogdanIrimie approved these changes

Assignees
No one assigned
Labels
enhancement New feature or enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kozl @puppetlabs-jenkins @puppetcla @BogdanIrimie @oanatmaria