-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
picoev: refactor #20558
picoev: refactor #20558
Conversation
@enghitalo please avoid refactoring/renaming in the same PR, as functional changes. I know that it is sometimes unavoidable, but here, more than half of the changes are renames. Intermixing both in the same PR makes it much harder to read and approve, and the cost in time is not linear, especially if there are problems found. You (and I) will have to read those diffs over and over and over again, till everything is good to merge, while if those were 2 separate PRs (one pure refactoring, and another with potentially breaking, but surely functional changes), the pure refactoring one would have been approved and merged pretty much immediately, and the diff for the second one, would have been much smaller and focused. |
Think of it as wading through a pile of straw (the renames), to search for a pearl, or a needle potentially hidden in it. |
@spytheman Thank you for your review and tips. I will try finish it next week |
This reverts commit 96336a2.
No description provided.