-
Notifications
You must be signed in to change notification settings - Fork 11
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]: following elixir conventions #57
base: master
Are you sure you want to change the base?
Conversation
@Dwctor Can you tell me if the changes brought by this PR (which is still a draft) have any negative impact on the compilation process? I'm still trying to understand more complex parts of the code and it's not entirely clear how everything relates. |
@sleipnir So far there is no changes in the compilation process! If you want to test it, make sure you have all of the dependencies and run: ps: glibc version 2.41 seems to be bugged, reverting to 2.40 fixes it. Feel free to contact me as far as what parts relate to what others. I'll be more active starting next week. ps2: if you are in an arch-based distro, please do not reboot/restart your machine before returning glibc to it's proper version, or you will have to chroot into it from a bootable pendrive to fix it! |
Awesome @Dwctor |
This PR reorganizes the project structure to better align with Elixir/OTP conventions while maintaining eBPF compiler specifics. Key improvements: Why Elixir Conventions Matter Key Changes:
Sorry for large PR |
ping @Dwctor |
@sleipnir I'm looking over the changes now. Most things seem to be great! I'd just like you thoughts on these so we are on the same page:
I don't agree with the following:
I won't be able to finish looking through everything today, so I'll leave more comments tomorrow, when I'll also test the compilation. |
Thanks for your quick response
Answer: Naming conventions. It's very strange for an Elixir developer to use that syntax, given that it's a module and the default convention is CamelCase.
Answer: Guards are still utilities, the proposal was Decomposed utils.ex into domain-specific helpers
Answer: The question is, will we be able to have other types of analysis in the future? If so, the name of the folder as I suggested makes the most sense. Regarding the name of the Liveliness module, I really have no idea what name would be best, but when I wrote it, it seemed to be part of a semantic analysis, if it is not then we can rename it as we see fit. Give me a name other than "Analysis" and I think it will be the best.
<3 |
This is only used by Honey Potion, therefore runtime, but the module name has not changed, only the folder so that it is not directly in the project root. Any name would work here I think, do you suggest one? |
No description provided.