-
Notifications
You must be signed in to change notification settings - Fork 14
Add Matroschka processing #381
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 Matroschka processing #381
Conversation
PR is ready for review. Responses to pointers you gave me in discord:
I apologize for however many I've missed, but I went through the things you told me to look for there and caught the ones I noticed
All non-todo comments seem ok to me after a pass
I think I've caught all these
Every remaining TODO (save for maybe 1 or 2) is a personal comment, because it's me wanting confirmation or otherwise from you. Assume every TODO I've left is asking you something, unless it really seems like it isn't. Otherwise, there shouldn't be any other personal comments
There's probably some naming issues, but I wouldn't be able to determine them on my own.
My current access levels definitely don't make sense. I hate to do this, but this is probably singlehandedly my greatest weakness in terms of programming; every time I make something private or static it winds up completely destroying my program, and every fix feels awful. I'll need your advice on what publics can be made private and what non-statics can be made static upon your review
I think I'm using everything I can that I should, but let me know if you see any areas where some more extensions would be useful.
I think I've done this
Just went through and did this.
Just went and did that.
Did that
Did that
It's currently in the Sections region; let me know if I can move it to the extension properties section or not. |
PR relies on SabreTools/SabreTools.Serialization#23
Like that PR currently is as I'm writing this message, this is a draft for now. There isn't much point looking it over until I un-draft it, although this PR is admittedly far simpler than the Serialization one, so there won't be as much for me to fix/change.
I will un-draft it at the same time as the serialization one, as it probably helps to understand why I want certain things in the serialization one.