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

Syslog -> Added colour priority to the label column #11607

Merged
merged 1 commit into from Jun 5, 2020

Conversation

TheGreatDoc
Copy link
Contributor

Now the label column have the corresponding colour based on priority.

Feel free to comment if I should change any colour<->priority.

Before:
image

After:
image

DO NOT DELETE THE UNDERLYING TEXT

Please note

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

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

* @param int $syslog_priority
* @return string $syslog_priority_icon
*/
private function priorityLabel($syslog_priority)
Copy link
Member

Choose a reason for hiding this comment

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

This should be a common function, if you search for label-danger in the code you see how many places does the same thing over and over :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, I took it from the EventlogController.

But what is best, one func with lots of conditionals or different functions even doing "the same"?

For example, eventlog handles with numeric "severity" while syslog does by string "priority".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Jellyfrog do you think I have to do a different approach with what I've said about different conditions?

@TheGreatDoc TheGreatDoc mentioned this pull request May 21, 2020
2 tasks
@murrant murrant merged commit c232cf0 into librenms:master Jun 5, 2020
@TheGreatDoc TheGreatDoc deleted the syslog-colour_by_level branch June 24, 2020 06:09
@murrant
Copy link
Member

murrant commented Jul 3, 2020

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-65-release-changelog-june-2020/12687/1

1 similar comment
@murrant
Copy link
Member

murrant commented Jul 3, 2020

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/v1-65-release-changelog-june-2020/12687/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants