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

Change Dell iDrac from Server to Appliance #8642

Merged
merged 3 commits into from May 8, 2018

Conversation

Projects
None yet
6 participants
@theherodied
Copy link
Contributor

theherodied commented Apr 30, 2018

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@theherodied

This comment has been minimized.

Copy link
Contributor Author

theherodied commented Apr 30, 2018

@laf wasn't sure if we needed test data for this?

Thinking this should have an advance warning before being merged in case people had the appliance category in a rule.

Also, I wasn't sure if iLO should be server or if iDrac should be appliance. I went with server for both.

@Rosiak

This comment has been minimized.

Copy link
Contributor

Rosiak commented May 2, 2018

I'm fine with either.
@librenms/reviewers ?

@kkrumm1

This comment has been minimized.

Copy link
Member

kkrumm1 commented May 2, 2018

Sounds good to me.

@murrant

This comment has been minimized.

Copy link
Member

murrant commented May 5, 2018

IMHO, ILO and iDRAC are appliances but supply server data. I'm fine with either, but I agree it should be consistent.

@theherodied theherodied changed the title Change HPE iLO from Appliance to Server Change Dell iDrac from Server to Appliance May 5, 2018

@theherodied

This comment has been minimized.

Copy link
Contributor Author

theherodied commented May 5, 2018

Agreed. Will change iDrac instead and update PR.

@laf

This comment has been minimized.

Copy link
Member

laf commented May 7, 2018

You didn't need fresh test data but we had no OS data saved from what we had so I've added that and updated this PR.

@scrutinizer-notifier

This comment has been minimized.

Copy link

scrutinizer-notifier commented May 7, 2018

The inspection completed: No new issues

@theherodied

This comment has been minimized.

Copy link
Contributor Author

theherodied commented May 7, 2018

Thanks @laf. I think this is gtg then.

@murrant

murrant approved these changes May 8, 2018

Copy link
Member

murrant left a comment

LGTM

@murrant murrant merged commit d204a1b into librenms:master May 8, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

slashdoom added a commit to slashdoom/librenms that referenced this pull request May 8, 2018

Change Dell iDrac from Server to Appliance (librenms#8642)
* Change HPE iLO from Appliance to Server

* reverted ilo to appliance, set idrac to appliance

* Added OS test data

slashdoom added a commit to slashdoom/librenms that referenced this pull request May 8, 2018

Change Dell iDrac from Server to Appliance (librenms#8642)
* Change HPE iLO from Appliance to Server

* reverted ilo to appliance, set idrac to appliance

* Added OS test data

TheMysteriousX added a commit to TheMysteriousX/librenms that referenced this pull request May 20, 2018

Change Dell iDrac from Server to Appliance (librenms#8642)
* Change HPE iLO from Appliance to Server

* reverted ilo to appliance, set idrac to appliance

* Added OS test data

@laf laf added the Device 🖥 label May 30, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jul 29, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.