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

fix: ePMP gps state #8575

Merged
merged 2 commits into from Apr 17, 2018
Merged

fix: ePMP gps state #8575

merged 2 commits into from Apr 17, 2018

Conversation

pheinrichs
Copy link
Contributor

fixes: #8567

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

@pheinrichs
Copy link
Contributor Author

Any idea why this would have stopped working? The above does resolve, just curious is all.

@laf
Copy link
Member

laf commented Apr 17, 2018

The {{ $index }} at the end isn't needed (never has and I'm not sure if it would have done anything or caused issues).

Specifying the index of 0 shouldn't be needed and {{ $index }} should work fine there - are you sure you needed to change that?

@murrant
Copy link
Member

murrant commented Apr 17, 2018

@laf {{ $index }} isn't needed here, I allowed it in the new processors stuff. I'll try to make it all consistent sometime.

@pheinrichs
Copy link
Contributor Author

Gotcha, so it was probably running 21.1.1.7.{{$index}}.{{$index}}'. So it should be okay like this?

@laf
Copy link
Member

laf commented Apr 17, 2018

Yes that should be fine although I think you should do index: {{ $index }} instead of index: 0

@laf
Copy link
Member

laf commented Apr 17, 2018

@laf {{ $index }} isn't needed here, I allowed it in the new processors stuff. I'll try to make it all consistent sometime.

@murrant Would be good to standardise them although I don't think processors supports not having the {{ $index }} right now - didn't work for me earlier at least.

@laf
Copy link
Member

laf commented Apr 17, 2018

@pheinrichs Does that change to index still work for you so we can merge?

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@pheinrichs
Copy link
Contributor Author

@laf Rediscovered and polled were a success. Good to go.

Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

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

LGTM

@laf laf merged commit 052b9d1 into librenms:master Apr 17, 2018
@pheinrichs pheinrichs deleted the fix-epmp-gps branch May 16, 2018 18:19
TheMysteriousX pushed a commit to TheMysteriousX/librenms that referenced this pull request May 20, 2018
* Update ePMP gps state

* Update index in yaml
@lock lock bot locked as resolved and limited conversation to collaborators Jul 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ePMP GPS up in radio web interface, down in Libre
4 participants