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.

Sign up for GitHub

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

Jump to bottom

[internode_usage] New plugin for Australian ISP Internode #980

Merged
merged 2 commits into from Jul 22, 2019
Merged

[internode_usage] New plugin for Australian ISP Internode #980

merged 2 commits into from Jul 22, 2019

Conversation

shtrom
Copy link
Contributor

@shtrom shtrom commented Apr 12, 2019

Signed-off-by: Olivier Mehani shtrom@ssji.net

@shtrom

This comment has been minimized.

Sign in to view
@shtrom shtrom force-pushed the isp/internode branch 5 times, most recently from 164ea25 to f167eb6 Compare April 19, 2019 10:21
@sumpfralle
Copy link
Collaborator

Thank you for working on this.

Just for the technical detail: feel free to squash the fixup commits into their parents (e.g. git rebase -i --autosquash) and force-push your branch.

@shtrom
Copy link
Contributor Author

shtrom commented May 27, 2019

Yup, I'm still working on it and testing locally. I thin I have fixes for a few of the bugs I found.

@shtrom
Copy link
Contributor Author

shtrom commented Jun 6, 2019

Some screenshots:
internode_current-day
internode_daily-week
internode_usage-month

@shtrom
[network/upnpc_] don't let the shell expand ${graph_period}
92db831 
Also, update license and add SPDX identifier

Signed-off-by: Olivier Mehani <shtrom@ssji.net>
@shtrom
[internode_usage] New plugin for Australian ISP Internode
691e52b 
* Add BUGS and SPDX license
* Add dirtyconfig support
* Use ideal usage as warning
@sumpfralle
Copy link
Collaborator

Just a sidenote: 92db831 seems to belong to a separate pull request. Additionally I would appreciate a more descriptive comment (e.g. the single quotes around the here-doc delimiter disable variable substitution (e.g. for '${graph_period}')). I had to struggle a bit until I understood the context ...

@shtrom
Copy link
Contributor Author

shtrom commented Jun 6, 2019

Ah, yeah, I bundled it because it was a quick fix. It was on purpose.

@sumpfralle
Copy link
Collaborator

Ah, yeah, I bundled it because it was a quick fix. It was on purpose.

OK - a nifty plan! :)
(agreed)

@sumpfralle
Copy link
Collaborator

To avoid any misunderstanding: do you think it is ready for merge?
(your last comment from May indicated, that you are still working on it)

@shtrom
Copy link
Contributor Author

shtrom commented Jul 22, 2019

Ah, yeah, this one's good to go, it's been chugging along for a few month for me 👍

@sumpfralle sumpfralle merged commit 7f77bdf into munin-monitoring:master Jul 22, 2019
@sumpfralle
Copy link
Collaborator

Great!

@shtrom shtrom deleted the isp/internode branch July 23, 2019 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@shtrom @sumpfralle