-
Notifications
You must be signed in to change notification settings - Fork 419
Fixes #704 #709
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
base: master
Are you sure you want to change the base?
Fixes #704 #709
Conversation
|
Ou of context request but ngl you seem like a person who actually understands the whole working of the script i would just like to request you if you also try to fix the drops progress bar feature. I tried fixing it using chatgpt and all that stuff but it didn't worked even made it an issue in this repo but I think the owner isn't active nowadays so maybe you could try to fix it if you're fine with it 😅 |
I'm happy you think I have a decent knowledge of the codebase, unfortunately I don't really use the drops feature so my working knowledge of that isn't great. I'm currently working on something else for the project (#722) so I can't look into it now. Might be a while for me so your best bet is to try something yourself and see if you can get it working. |
|
Oh alr no issues about that then but if you're working on something else rn that's great but then I have another request for you that instead of pushing changes into a pr here just make a fork of this and push into there cuz ngl this repo is pretty dead now and as of rn many people have proposed some fixes which still haven't been merged and some of them even just closed their pull requests because of this. With a new and active fork atleast the changes proposed by community would be merged. Again this is just a suggestion 😄 |
Even the last actual update with any new features and improvements was last year i believe |
I do have a fork that's the base for my PRs here. Anyone can make their own build using that as a base, there's no automated build system since it's only intended to be used for developing new features for this repo. You could also try making your own fork, implementing a fix, and making an upstream PR back to this repo. This would allow you to run the project using your own fix branch as a base. This is actually how I run the project as I like to use a custom prediction system as per #659.
The last update that fixed a major issue was 3 months ago, although I'll give you that this wasn't a feature update. The last feature update was almost a year ago. Another option to help enable more frequent updates would be to add some project collaborators. This should enable any invited community member to make changes when the repo owner or other collaborators are unable to make time for it. |
Adds an abstract base class for all log event hook classes. Adds LoggerSettings.hooks to enable users to add as many of these as they need to their configs. Updates LoggerSettings constructor type hinting to use "|" instead of "or". Moves logger.py log validation functions into their respective hook class. Abstracts the attribute based log validation into an intermediary hook class method. Updates the README.md and example.py. Potentially fixes a bug with how DROP_STATUS Events worked with Pushover.
|
@rdavydov is there anything left to address before this PR can be merged? would love to see this feature! |
Description
Creates an abstract class
EventHookfor the log hook classes (Discord,Gotify,Matrix,Pushover,Telegram, andWebhook) and adds alist[EventHook]to theLoggerSettings. This enables users to specify as many hooks as required in their settings.I've kept the old named hooks in the settings object to maintain backwards compatibility with existing user's settings.
This also potentially fixes a bug with the
Pushoverhook. TheDROP_STATUSlogs get combined into a single message for the other hooks, I think this got missed when thePushoverhook was written but nobody has made an issue about it so maybe it's fine.I considered making the hooks into log handlers directly but that would have meant either inefficiently duplicating the work the log formatter does or having to rework the log formatter. It would also make it more difficult to combine those
DROP_STATUSlogs. This can still be done later if people think it would be better.There is potentially another bit of code that could be abstracted, the hooks'
sendmethods all first check if the giveneventis in the hook'sself.events. This could be moved to thevalidate_recordstep to avoid having to redo this step for each hook. I've left it alone for now because I wanted to avoid touching thesendmethods if possible.Finally, I think the reason
Gotify.pylooks like I've replaced the whole file is because it originally had windows line endings, I reformatted the file to use unix ones because git was complaining about it.Fixes #704
Type of change
How Has This Been Tested?
I added multiple Discord hooks to the list with different event filters and saw the desired events went to the correct hooks. I also tested using the existing named hook alongside this and it also worked as expected. I did not test with the other hook classes.
Checklist: