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

webui: Added Oxidized config validator #7983

Merged
merged 4 commits into from Jan 11, 2018

Conversation

Projects
None yet
3 participants
@laf
Member

laf commented Dec 29, 2017

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

I'm happy for this not to be part of LibreNMS and I'll just chuck it on my site for people to use

@laf laf added the WebUI label Dec 29, 2017

@murrant

This comment has been minimized.

Member

murrant commented Dec 29, 2017

Interesting, I'll take a peak sometime.

@murrant

This comment has been minimized.

Member

murrant commented Jan 8, 2018

Works :)

Needs positive feedback when the config is ok.
The textarea is too tall, I have to scroll a lot to get do the submit button.

@murrant

This comment has been minimized.

Member

murrant commented Jan 8, 2018

Any thoughts of putting a button on the oxidized page instead of the menu?
I'm not convinced one is better than the other, just a thought.

@laf

This comment has been minimized.

Member

laf commented Jan 8, 2018

Just pushing a commit that will reduce the box height and also adds feedback when the config is ok.

I'm not fussed either way on where the link actually is, possibly would be more hidden if it was within the other page though.

@murrant

murrant approved these changes Jan 9, 2018 edited

I'm not decided either way regarding the menu entry vs button.

Is it worth considering that you won't use this often after initial setup?

@laf

This comment has been minimized.

Member

laf commented Jan 9, 2018

That's a good point. I'll move it tomorrow.

@laf

This comment has been minimized.

Member

laf commented Jan 10, 2018

Slight rejig of location for the link.

image

Also added small bit of blurb in docs for it.

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 10, 2018

The inspection completed: 674 Issues, 25 Patches

@murrant murrant merged commit d26d142 into librenms:master Jan 11, 2018

2 checks passed

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

@laf laf deleted the laf:web/oxidized-cfg branch Jan 11, 2018

@lock

This comment has been minimized.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

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