Skip to content

Chore: Refactoring annotation mode logic into controllers#7

Closed
pramodsum wants to merge 2 commits intobox:masterfrom
pramodsum:annotations
Closed

Chore: Refactoring annotation mode logic into controllers#7
pramodsum wants to merge 2 commits intobox:masterfrom
pramodsum:annotations

Conversation

@pramodsum
Copy link
Copy Markdown
Contributor

No description provided.

@pramodsum pramodsum force-pushed the master branch 18 times, most recently from b5efd2a to 3521db1 Compare October 25, 2017 02:47
Copy link
Copy Markdown
Contributor

@JustinHoldstock JustinHoldstock left a comment

Choose a reason for hiding this comment

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

That's some serious cleanup! See my comment, then lgtm (and tests)

*/
getThreadByID(threadID) {
let thread = null;
Object.keys(this.threads).forEach((page) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you optimize this with .some() instead of .forEach()? There's no point in iterating the ENTIRE list if you've already found the item you're looking for

@pramodsum
Copy link
Copy Markdown
Contributor Author

Will re-open in a separate PR w/ the merge conflicts sorted out

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants