Skip to content

benchmark: clean up common.js#662

Closed
brendanashworth wants to merge 1 commit into
nodejs:v1.xfrom
brendanashworth:clean-up-benchmark-common
Closed

benchmark: clean up common.js#662
brendanashworth wants to merge 1 commit into
nodejs:v1.xfrom
brendanashworth:clean-up-benchmark-common

Conversation

@brendanashworth
Copy link
Copy Markdown
Contributor

This commit cleans up benchmark/common.js with a few generic changes
such as the following:

  • declare all require()'d libraries at the top instead of in the
    middle
  • add some empty whitespace where it helps readability
  • changes ambiguous variable names
  • standardizes most if / else blocks
  • missing semicolons

This is sort of the prep for an up and coming PR I'm writing, but I just couldn't work with the code as it was.

@yosuke-furukawa
Copy link
Copy Markdown
Member

LGTM

@micnic
Copy link
Copy Markdown
Contributor

micnic commented Jan 30, 2015

LGTM

what sort of changes do you want to add with the upcoming PR?

Comment thread benchmark/common.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor style nit: I don't think it's a hard rule but imports tend to be sorted in alphabetical order.

This commit cleans up `benchmark/common.js` with a few generic changes
such as the following:

- declare all `require()`'d libraries at the top instead of in the
  middle
- add some empty whitespace where it helps readability
- changes ambiguous variable names
- standardizes most if / else blocks
- missing semicolons
@brendanashworth brendanashworth force-pushed the clean-up-benchmark-common branch from aee5848 to 5e16c62 Compare January 31, 2015 02:14
@brendanashworth
Copy link
Copy Markdown
Contributor Author

Fixed from @bnoordhuis's comment. @micnic maybe some special plotting goodness 😄

@bnoordhuis
Copy link
Copy Markdown
Member

LGTM

brendanashworth added a commit that referenced this pull request Feb 2, 2015
This commit cleans up `benchmark/common.js` with a few generic changes
such as the following:

- declare all `require()`'d libraries at the top instead of in the
  middle
- add some empty whitespace where it helps readability
- changes ambiguous variable names
- standardizes most if / else blocks
- missing semicolons

PR-URL: #662
Reviewed-By: Nicu Micleușanu <micnic90@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
@Qard
Copy link
Copy Markdown
Member

Qard commented Feb 2, 2015

Landed in 89dd8e0

@Qard Qard closed this Feb 2, 2015
@brendanashworth brendanashworth deleted the clean-up-benchmark-common branch March 8, 2015 20:11
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.

5 participants