v2.1.x Chore - review feedback

[reactive-firewall]

context 🙉

🤔 looks like there will be no merging of the two projects due to the differences in code style 🤷

But I did get honest feedback

Hi @reactive-firewall , thanks for reaching out! I appreciate the offer, but after reviewing your project, I think there's too big of a difference between our coding styles for us to collaborate.

I have noticed that you make heavy use of AI in your work, which is a technology I have concerns about. While AI is capable of some impressive things, it is also prone to making mistakes, some of which me and my colleagues have found when reviewing your codebase. It also has a tendency to be too verbose and generate "word salad", hiding important information amid large amounts of text and comments and making code harder to understand, review, and audit.

At multicast expert, we are committed to producing clear, commented code with high quality tests and documentation that make it easy to work with. I do not believe that use of AI, in its current form, is compatible with those goals.

If you are serious about continuing with your project, I do have some suggestions. Multicast is a hard problem, especially setting the low level socket options and working with multiple network interfaces. multicast_expert provides a working implementation of this stuff, so you really shouldn't need to do it yourself. Your project can and should build on top of multicast_expert and focus on stuff that it doesn't do, such as the command line interface and a higher level API. I would also suggest improving your testing -- we noticed that your tests tend to focus on the basic stuff like importing things, but are spotty when it comes to testing real functionality. Last but not least, we recommend introducing static typing and type annotations for your API, as this will make it easy for others to check their types and help IDEs type hints for your functions.

Originally posted by @multiplemonomials in multiplemonomials/multicast_expert#24

While I'm sorry to hear that there will be no collaboration on the project 🙇 I want to @multiplemonomials for the feedback.

Takeaways from exploring the collaboration

Concerns to address

• codebase looks like it makes heavy use of AI (which is not intended as AI is INDEED prone to mistakes)

  • this underlines the importance of the GHI Harden code review protocol #367
  • The role of AI should be clarified in collaboration guide and other documents.

• type annotations are expected by some developers (even if python does not actually enforce types)

  • reopen Migrate to Python 3.9+ language features #134 and review type hints
  • documentation should indeed mention CEP-8 (not to be confused with PEP 8)

Next Steps

• ensure this ticket is not AI based (while AI improves quality of life, it should not overshadow me in my own portfolio ☹️ )
• look into the tests used by multicast_expert and use them to enhance criteria for mutlicast where relevant to project goals
  • ipv6
  • interface logic
  • windows support? (at least should consider)
• review codebase for clearity from perspective of developers
• update FAQ with mention of multicast_expert as alternative (especially for windows)