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

Remote monitoring using tinc VPN #5122

Merged
merged 4 commits into from Dec 9, 2016

Conversation

Projects
None yet
5 participants
@florianbeer
Contributor

florianbeer commented Dec 6, 2016

Please note

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

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Dec 6, 2016

Auto-Deploy finished, Test PR at http://5122.ci.librenms.org or https://5122.ci.librenms.org

@Gorian

Mostly looks good. I like tinc :) A few pieces of feedback: either prefix your commands with "#" or don't. But, you don't seem to hold to one style or the other - I'd say just leave the "#" off the commands.

Also, a general FYI, you can choose the syntax highlighting on bash scripts like this:

```bash
#/usr/bin/env bash
echo "hello world"
```

to get

#/usr/bin/env bash
echo "hello world"

florianbeer added some commits Dec 8, 2016

@florianbeer

This comment has been minimized.

Contributor

florianbeer commented Dec 8, 2016

Thanks for your feedback @Gorian

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Dec 8, 2016

Auto-Deploy finished, Test PR at http://5122.ci.librenms.org or https://5122.ci.librenms.org

1 similar comment
@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Dec 8, 2016

Auto-Deploy finished, Test PR at http://5122.ci.librenms.org or https://5122.ci.librenms.org

@laf

This comment has been minimized.

Member

laf commented Dec 9, 2016

I've got some small changes which I tried to push to your branch, can you confirm that this option is enabled on the bottom right of this page for you:

image

It will allow me to make the changes directly.

@florianbeer

This comment has been minimized.

Contributor

florianbeer commented Dec 9, 2016

Yes, I deliberately left that enabled so you could make changes. Strange that it doesn't work.

I've un-checkd and re-checked it. Maybe Roy's tip sometimes really does work (if you've seen the IT crowd you'll get it).

@laf

This comment has been minimized.

Member

laf commented Dec 9, 2016

Sorted, you can see my changes in the commit I pushed.

Basically I removed # from commands - we don't do this anywhere else so wanted to standardise it. I renamed the file and added it to the menu bar so people can get to it.

Please check you are happy and if so I'll publish on blog + merge.

@florianbeer

This comment has been minimized.

Contributor

florianbeer commented Dec 9, 2016

I am happy 👍

Thank you!

@LibreNMS-CI

This comment has been minimized.

LibreNMS-CI commented Dec 9, 2016

Auto-Deploy finished, Test PR at http://5122.ci.librenms.org or https://5122.ci.librenms.org

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Dec 9, 2016

The inspection completed: No new issues

@laf laf merged commit 163a171 into librenms:master Dec 9, 2016

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@florianbeer florianbeer deleted the florianbeer:patch-1 branch Dec 9, 2016

VimCommando added a commit to VimCommando/librenms that referenced this pull request Jan 4, 2017

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