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

initial PR for librenms #1704

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

svennd
Copy link

@svennd svennd commented Jan 23, 2025

✍️ Description

Librenms is a fully featured network monitoring system that provides a wealth of features and device support.


  • Related Discussion: #1590

✅ Prerequisites

The following steps must be completed for the pull request to be considered:

  • Self-review performed (I have reviewed my code to ensure it follows established patterns and conventions.)
  • Testing performed (I have thoroughly tested my changes and verified expected functionality.)

🛠️ Type of Change

Please check the relevant options:

  • [] Bug fix (non-breaking change that resolves an issue)
  • [] New feature (non-breaking change that adds functionality)
  • [] Breaking change (fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts)

📋 Additional Information (optional)

Given this is my first script, I would appreciate any (contructive) feedback ! I created it, cause I would like to use it myself. But of course the entire community can use it :) I expect to still improve some small things.

After the web installation, you get to this screen : the 2 warnings, ofcourse can be resolved by adding devices to monitor and set the specific url (domain) or ip. (minor things)
librenms_clean

@svennd svennd requested a review from a team as a code owner January 23, 2025 20:08
@github-actions github-actions bot added new script A change that adds a new script website A change to the website labels Jan 23, 2025
@github-actions github-actions bot added maintenance Code maintenance or general upkeep of the project high risk A change that can affect many scripts labels Jan 23, 2025
@MickLesk
Copy link
Member

Remove Change on build.func please

@svennd
Copy link
Author

svennd commented Jan 23, 2025

Remove Change on build.func please

Yes sorry, I added it cause I figured out how to "fix" timezone warning.

@MickLesk MickLesk removed the high risk A change that can affect many scripts label Jan 24, 2025
Copy link
Member

@michelroegl-brunner michelroegl-brunner left a comment

Choose a reason for hiding this comment

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

Overall formating is bad, also remove comments.

ct/librenms.sh Outdated
msg_error "No ${APP} Installation Found!"
exit
fi
msg_ok "${APP} automatically updated!"

Choose a reason for hiding this comment

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

The app updates on its own without user input?

install/librenms-install.sh Show resolved Hide resolved

# Copyright (c) 2021-2025 community-scripts ORG
# Author: SvennD
# License: MIT

Choose a reason for hiding this comment

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

License in one line

$STD useradd librenms -d /opt/librenms -M -r

# set timezone
$STD timedatectl set-timezone Etc/UTC

Choose a reason for hiding this comment

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

Formating?


msg_info "download librenms"
cd /opt
$STD git clone https://github.com/librenms/librenms.git --depth 1

Choose a reason for hiding this comment

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

No git clone if possible. Download latest release. have a look at the guides in the wiki.

Comment on lines 65 to 66
$STD systemctl enable librenms-scheduler.timer
$STD systemctl start librenms-scheduler.timer

Choose a reason for hiding this comment

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

Suggested change
$STD systemctl enable librenms-scheduler.timer
$STD systemctl start librenms-scheduler.timer
systemctl enable -q —now librenms-scheduler.timer

json/librenms.json Outdated Show resolved Hide resolved
svennd and others added 3 commits January 27, 2025 10:11
@svennd
Copy link
Author

svennd commented Jan 27, 2025

Thanks for the review @michelroegl-brunner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Code maintenance or general upkeep of the project new script A change that adds a new script website A change to the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants