-
Notifications
You must be signed in to change notification settings - Fork 74
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
Refactor of IPC and error handling #329
Conversation
The codebase has indeed become rather unruly. I've tried refactoring it a bit to make it easier to parse, but I was already planning another refactor to make this clearer in general. That said, there are a few things I would like to point out about this: Overall, since I was planning on doing this anyway, it would be a welcome improvement. But I do think it would be better to separate it into different PRs so we could review it one step at a time. I've made some gigantic PRs lately, but they were often changing a lot of the code's internal logic all at once, so it was hard to compartmentalize them. It seems to me that this PR, on the other hand, could be divided in smaller chunks. This would also have the benefit that, should you try changing something I don't like, we can just ignore that PR and still use all the others. EDIT: if your problem is understanding the codebase, might I recommend writing documentation for the functions instead? That would still be immensely helpful, would make you understand everything better, would make it possibly easier to do later refactors like what you are thinking, and would make the codebase easier to understand. Though it is much less exciting than a large refactor. |
Ok, I'll try to split this into multiple PRs. I wanted Should I close this PR and start sending few small ones, given that this is reaching hundreds of lines already? |
I think that would be ideal, yeah.
You can use |
Alright, be right back. |
Hello,
I wanted to learn from this codebase and all too often got stuck when reading the code.
I would like to refactor it to (what I consider) more idiomatic Rust. My goal was to organize
IPC and friends around a single data structure with associated functions instead of passing
file descriptors around and then to look into the large memory usage. I don't want to adjust
behaviour (yet).
@LGFae would you be OK with this effort? I think it would be fairly major refactor, with preview
shown in these first four commits.
Thank you very much for your time.