Skip to content
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

Added 'cc' to lint the C++ code on 'npm lint' #1501

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

IvanSanchez
Copy link
Contributor

The whitespace fumble at https://github.com/node-serialport/node-serialport/pull/1483/files#r168511525 made me think that node-serialport uses eslint for linting the javascript code, but doesn't do anything for the c++ code.

This change adds cc as a dependency, which in turn runs the python cpplint script. All of the c++ code, except for some bits in the osx headers, seem to pass the linting.

@reconbot
Copy link
Member

Get it passing and will happily merge 👍

@IvanSanchez
Copy link
Contributor Author

Apparently the build fails because some file is renamed into src/serialport_linux.cpp, but right now I can't see what's making that happen 😐

@IvanSanchez
Copy link
Contributor Author

ZOMG, and some travis tests are running the c++ linter over the whole nodejs source code tree, failing everywhere 🤦‍♂️

@IvanSanchez
Copy link
Contributor Author

IvanSanchez commented Feb 26, 2018

OK, the linux/mac builds pass 🎉, but apparently I made some change that made the win32 builds fail 😢:

  C:\projects\node-serialport\node_modules\nan\nan.h(1568): note: see declaration of 'Nan::Callback::Call'
..\src\serialport_win.cpp(181): error C2440: 'static_cast': cannot convert from 'HANDLE' to 'int' [C:\projects\node-serialport\build\serialport.vcxproj]
  C:\projects\node-serialport\node_modules\nan\nan.h(1568): note: see declaration of 'Nan::Callback::Call'
..\src\serialport_win.cpp(616): error C2440: 'reinterpret_cast': cannot convert from 'const GUID *' to 'GUID *' [C:\projects\node-serialport\build\serialport.vcxproj]

@IvanSanchez
Copy link
Contributor Author

🎉

@reconbot
Copy link
Member

Can you rebase?

Make linter happier.

Make linter happier.
@codecov-io
Copy link

codecov-io commented Feb 27, 2018

Codecov Report

Merging #1501 into master will increase coverage by 0.76%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1501      +/-   ##
=========================================
+ Coverage   79.04%   79.8%   +0.76%     
=========================================
  Files          21      19       -2     
  Lines         940     822     -118     
  Branches      170     165       -5     
=========================================
- Hits          743     656      -87     
+ Misses        197     166      -31
Impacted Files Coverage Δ
lib/bindings/auto-detect.js 38.46% <0%> (-61.54%) ⬇️
lib/util.js 83.33% <0%> (-8.34%) ⬇️
lib/bindings/win32.js
lib/bindings/darwin.js

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 093c85d...cbda993. Read the comment docs.

@reconbot reconbot merged commit 59960a3 into serialport:master Feb 28, 2018
@reconbot
Copy link
Member

This is great, thank you!

@IvanSanchez IvanSanchez deleted the lint-c branch February 28, 2018 09:39
oteku pushed a commit to Cutii/node-serialport that referenced this pull request Apr 9, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants