For review: Replace \\\\l with \l on GPRINT lines #4882

Merged
merged 1 commit into from Oct 25, 2016

Projects

None yet

5 participants

@jquagga
Contributor
jquagga commented Oct 25, 2016

Please note

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

This is a PR for review. @laf mentioned on IRC about submitting this. On many GPRINT lines, they end with \\\\l instead of \l. This on occasion causes the graph to produce an extra \ at the end of the graph legend lines. Its unclear to me why the ending wouldn't be just \l.

This has shown up a few times recently in PRs. #4827 fixed mempool and I submitted #4828 to fix the wifi clients graph.

This patch should change every file in includes/graphs to utilize \l instead of \l. It probably needs further testing. I don't seem to have any issues after changing this, but again it's not clear to me why 4 \ were used in the first place.

@jquagga jquagga Replace \\\\l with \l on GPRINT lines
5355c19
@scrutinizer-notifier

The inspection completed: No new issues

@laf
laf approved these changes Oct 25, 2016 View changes
@laf
Member
laf commented Oct 25, 2016

Works for me. Anyone else care to test / comment @librenms/reviewers

@jquagga jquagga referenced this pull request Oct 25, 2016
Merged

fix: Toner graphs with invalid chars #4873

2 of 2 tasks complete
@paulgear
Member

I've got a few of these hanging around to submit myself. They have existed since ancient times (the very first revision of all those files in Observium svn). My best guess is that the rrdtool exec did some extra expansion at the time. But they are not needed. 👍 from me.

@laf laf merged commit 0ccbc5e into librenms:master Oct 25, 2016

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment