Skip to content

Various tweaks: #551

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

Merged
merged 1 commit into from
Mar 3, 2017
Merged

Various tweaks: #551

merged 1 commit into from
Mar 3, 2017

Conversation

sublimator
Copy link
Member

@sublimator sublimator commented Feb 28, 2017

Various tweaks:

  • Make options.port a number, after parsing the brokerUrl
  • Update some package.json dependencies (bar ws, left at 1.x)
  • Make travis script execute npm run ci to send codecov results
  • Add an .editorconfig file
  • Add nodejs 4.3.2 to the build matrix for AWS Lambda
  • Add C++11 compatible compiler for extensions

Mostly fairly innocuous changes, except the first one, which might give some people a little grief, so needs some consideration. The majority/meat of the tests all pass however (one needed adjusting to test the options.port set by a parsed url against number rather than a string). The port string/number inconsistency is something I noticed when porting the lib/* to TypeScript a while back.

I'd like to get that option consistently a number, then move ahead with #547

I added 4.3.2 to the build matrix just as a heads up. If it gets in the way of where people wanna go (e.g. updating ws to 2.x) we can just disable it. In the mean time, having the CI notify of breakages would be handy for AWS Lambda users, which has an ancient version of node, stuck in the past. Tsk Tsk AWS 👎

@sublimator sublimator requested a review from mcollina February 28, 2017 03:34
@codecov
Copy link

codecov bot commented Feb 28, 2017

Codecov Report

Merging #551 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
+ Coverage   92.58%   92.62%   +0.03%     
==========================================
  Files           8        8              
  Lines         634      637       +3     
  Branches      150      151       +1     
==========================================
+ Hits          587      590       +3     
  Misses         47       47
Impacted Files Coverage Δ
lib/connect/index.js 97.22% <100%> (+0.12%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b645b80...ab2b57d. Read the comment docs.

* Make options.port a number, after parsing the url
* Update some package.json dependencies (bar `ws`, left at 1.x)
* Make travis `script` execute `npm run ci` to send codecov results
* Add an .editorconfig file
* Add nodejs 4.3.2 to the build matrix for AWS Lambda
* Add C++11 compatible compiler for extensions
@sublimator
Copy link
Member Author

Anyone?

@mcollina
Copy link
Member

mcollina commented Mar 1, 2017

I'm currently traveling, I will review this asap.

@sublimator
Copy link
Member Author

sublimator commented Mar 1, 2017 via email

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@sublimator sublimator merged commit a3799c0 into mqttjs:master Mar 3, 2017
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