-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add official monitoring plugin #3160
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also overall question - can this plugin be extended by meters that other plugins could possibly add?
ArchiSteamFarm.OfficialPlugins.Monitoring/Localization/Strings.no-NO.resx
Outdated
Show resolved
Hide resolved
Just saw this comment:
As per my messages towars Archi earlier on our discord channel: In the current state it is not, but I have put some thoughts into it and I would like the input of other people. One option is to add an interface to ASF core, which the plugin could utilize. However, I am not a fan of adding features that are only used by a plugin that is not distributed by default. Another solution would be to add a public interface to the plugin project itself and tell people to add a reference to it. However, that requires this plugin to be installed even on setups that might not utilize monitoring capabilties. A third option is to somehow discover meters added by plugins in a way that neither adds a monitoring-specific interface to ASF core nor to this plugin. This would be my preferred option, but I am unsure of how to achieve this in a nice way. Of course we could load meter names from the configuration, but that would mean that users need to manually add the meters of alls their plugins and I am definitely not in favour of that. |
…y to fix the runtime exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another round, I don't believe there will be much more after that (if anything), and most of those changes are really misc - good job 🏆
ArchiSteamFarm.OfficialPlugins.Monitoring/Localization/Strings.resx
Outdated
Show resolved
Hide resolved
ArchiSteamFarm.OfficialPlugins.Monitoring/Localization/Strings.resx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last 3 things + please resolve conflicts 😎
… the source class)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, leaving this up for a few days in case anybody else wanted to review. Good job 🏆
Thanks! 🏆 |
Checklist
Changes
New functionality
Added official plugin for monitoring, which is NOT included with ASF by default.
Changed functionality
None
Removed functionality
None
Additional info
I am prepared to provide maintenance and support for this plugin, but I prefer this to be an OFFICIAL plugin, keeping my home setup supported in turn.
I've prepared the plugin to be updatable from GitHub and the pipelines to compile the plugin, but I've not yet changed the pipeline to zip and upload the binaries. The scripting skills of @JustArchi are superior to mine in every way and I am sure you would have thousands of complaints if I added that myself. It's faster if you do it in the post-PR cleanup (thanks in advance <3)
Thank you for considering the inclusion of this merge request.