Conversation
Using `map[string]func` is convenient, but its hard to read in Go and equally hard to test.
Fesaa
left a comment
There was a problem hiding this comment.
Heya, woah! Big PR, many thanks. Seeing a lot of better design!
I've pulled down, and skimmed over all files and added some comments. If you could resolve these, or respond if appropriate.
Once you've gone over these, I'll go over it all again.
Again, many thanks for the contribution and time you've spent working on this project.
|
Happy New Year @Fesaa! I was able to make all the changes you requested. If you see anything else let me know. The biggest change is the implementation of the interface for the discord notify service. This will enable you to add other notification services. |
|
Heya! Have you worked with https://github.com/go-playground/validator before? If so, are you willing to migrate the validation over to it? If not, no worries; I can do it. Same if you don't want to migrate. Your final commit seems to have broken a test, you'll have to fix this. I've added my final comments, as well. Lastly, if you could merge master before merging. Thank you! After that, I think we're ready to get it all merged in 🎉 Thanks for all your work on this update. ~ Amelia |
|
Hi! Yes I have used it/something like it. Let's add the validator to the next PR. I plan on doing a couple more, but wanted to get this larger refactor in first. I fixed the tests by adding TODOs and skipping the failing ones. I'm not sure I understand the logic of the modifiers since they will always iterate the |
@Fesaa this is a pretty big refactor and I'd understand if you don't want to accept it outright. Happy to make additional changes to get it to a place you'd feel comfortable merging.