-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Cleaned up blueprint.js #537
Conversation
crismali
commented
Apr 30, 2014
- removed unnecessary assignment
- removed unnecessary writefile function that just swapped arguments
- made double iteration in processFiles method a simple reduce
doesn't merge cleanly, mind rebasing? |
@@ -114,6 +109,13 @@ function markIdenticalToBeSkipped(info) { | |||
} | |||
} | |||
|
|||
function isNeedingConfirmation(collection, info) { |
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.
needsConfirmation
maybe?
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.
👍
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 naming is strange, as the function also has side effects.
I think @twokul's suggestion might make more sense.
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.
Agreed. Actually the old .filter()
-> .map()
way wasn't that bad.
@crismali - Can you squash the commits? |
@rjackson sure thing |
* removed unnecessary assignment * removed unnecessary writefile function that just swapped arguments * made double iteration in processFiles method a simple reduce * update changelog to reflect changes
@@ -114,6 +108,13 @@ function markIdenticalToBeSkipped(info) { | |||
} | |||
} | |||
|
|||
function gatherConfirmationMessages(collection, info) { |
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.
👍