Skip to content

Conversation

@Nathan-Gilbert6917
Copy link

No description provided.

@Nathan-Gilbert6917 Nathan-Gilbert6917 changed the title CVE-2013-3302 and CVE-2016-8630” CVE-2013-3302 and CVE-2016-8630 Nov 6, 2023
Copy link

@aisgbnok aisgbnok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was very good! 🎉 Compared to the others I reviewed, I feel you did a better job at actually distilling the jargon for readers. You explained things better instead of just providing technical facts/garbage, but your clarity could be improved.

Hopefully I was not too harsh. While I did not review everything for perfect accuracy, if I did not comment on something I most likely think it is fine.

I have requested some changes and provided some thoughts. You do not have to accept all of my suggestions and comments. I only ask that you take them into consideration.

Additional Notes

I think the description and mistakes answer are the most important for readers. The description explains what the CVE actually is, and the mistake answer explains why it happened and how not to have it happen to you. Take some extra time to craft those notes and improve your readability/clarity.

  • Try to improve your readability/clarity across the board. Some of your notes don't flow very well.
  • Is ssocket intentional? If not make sure to find and fix all instances, I didn't flag them all. #206 (comment)
  • In the future, try to maintain a "clean" git history. (Only change what is necessary.) #206 (comment)

Again, I can be harsh. Honestly, good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants