antrea-io / antrea

Kubernetes networking based on Open vSwitch
https://antrea.io
Apache License 2.0
1.65k stars 364 forks source link

replace math/rand #6674

Open kushal9897 opened 2 weeks ago

kushal9897 commented 2 weeks ago

6648

antoninbas commented 2 weeks ago

Please do not open a different PR for the same task in the future. We lose the review history for no reason.

This PR replaces #6659

antoninbas commented 2 weeks ago

@kushal9897 I do not think you took my advice from https://github.com/antrea-io/antrea/pull/6659#pullrequestreview-2297864918 in consideration. This should not be treated as a "search-and-replace" operation. You need to understand the changes introduced in math/rand/v2 by reading https://go.dev/blog/randv2 carefully, and in particular the Fixing the rest in math/rand/v2 section. Some issues are pretty obvious as the code no longer builds correctly with your changes. I recommend running make golangci locally to discover the most basic issues. Note that it may not be enough - it is possible that updating to math/rand/v2 will enable some improvements that can only be identified by reviewing usages of the math/rand package in the code base (cannot be identified through build failures). This is why reading that blog post carefully is important.

kushal9897 commented 2 weeks ago

Thank you for your feedback. I apologize for not fully considering your advice from #6659. You're right—I approached the changes more as a 'search-and-replace' operation without fully understanding the implications of the updates to math/rand/v2. I will carefully read the blog post, especially the 'Fixing the rest in math/rand/v2' section, and review the usage of the package in the codebase. I'll also run make golang locally to identify any further issues. I appreciate your guidance and will work on addressing this properly. Thank you again for your patience.