-
Notifications
You must be signed in to change notification settings - Fork 19
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
remove TODOs from code #306
Conversation
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.
Approved pending resolution of comment.
@@ -917,7 +915,6 @@ private void recordSelectionForCastVoteRecord( | |||
continue; | |||
} | |||
|
|||
// TODO: it's weird to have this check here |
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.
It seems like the rest of the stuff below here may have been part of the TODO? Does it make sense on its own?
// handle testing in unit tests, remove this assert, and add break statement
// at end of this for loop
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.
Oops. I just went ahead and did what @moldover suggested, except for adding a unit test. I'll create a new issue for that part.
fix silly (but innocuous) mistake in #306
We've also got FIXMEs in there that we should probably remove too. |
For each one, I either a) verified that it was no longer relevant, b) verified that an issue already existed (see #132), or c) created a new issue (see #304 and #305).