-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
lib: change == to === in linkedlist #9362
Conversation
LGTM if CI is ✅ |
LGTM |
1 similar comment
LGTM |
GREEN! CI IS GREEN! SECOND ONE IN A ROW! |
I think we should change the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change on line 19 LGTM. I don't know why the comment changed.
Apologies if the comment removal is a noisy change. I was working with @Trott and he mentioned we could remove the comment as the todo was not likely to happen. Admittedly I'm not very familiar with this part of the code base. I'm happy to resolve by;
|
I don't think fixing the TODO is viable because it is effectively public API. A comment should remain that the property names are opposite of what would be typically logical. |
Agree 100% with @Fishrock123: The |
Also removed a TODO comment that is no longer viable and left a note about the potentially confusing property naming convention for future readers.
I pushed edits updating the comment and commit message. |
Thanks! Landed in 62d8134. |
Also removed a TODO comment that is no longer viable and left a note about the potentially confusing property naming convention for future readers. PR-URL: #9362 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Also removed a TODO comment that is no longer viable and left a note about the potentially confusing property naming convention for future readers. PR-URL: #9362 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Also removed a TODO comment that is no longer viable and left a note about the potentially confusing property naming convention for future readers. PR-URL: #9362 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
lib, timers
Description of change
change == to === in linkedlist
Also removed an old TODO comment that likely won't happen anymore.