Introduce npm run lint command for cpp formatting #752
Conversation
| @@ -17,6 +17,7 @@ const chromiumRebaseL10n = require('../lib/chromiumRebaseL10n') | |||
| const createDist = require('../lib/createDist') | |||
| const upload = require('../lib/upload') | |||
| const test = require('../lib/test') | |||
| const lint = require('../lib/lint') | |||
There was a problem hiding this comment.
I think maybe you moved it to util but this is still around and it should get it from util instead.
There was a problem hiding this comment.
did you forget to add lint.js?
There was a problem hiding this comment.
Oops. I forgot to add lint.js. Added.
| console.log('linting ' + config.projects['brave-core'].dir + '...') | ||
| options.cwd = config.projects['brave-core'].dir | ||
| options = mergeWithDefault(options) | ||
| // git cl format checks rietveld.server is set. Just set null. |
There was a problem hiding this comment.
I think it is code review site of chromium project.
There was a problem hiding this comment.
I'm confused about what git cl has to do with lint. We want to call vendor/depot_tools/cpplint.py in some kind of push hook
There was a problem hiding this comment.
I didn't read full git_cl.py, but it seems it uses cpplint.py for lint internally. it seems it utilizes clang_format.py.
How about providing two kinds of formatting ways - npm run lint and git hook?
There was a problem hiding this comment.
git cl is utilities for chromium code review - https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-cl.html
It has many subcommands and we can easily use format or lint subcommand for our formatting
There was a problem hiding this comment.
but there's no changelist to run it on. We need it to run on directory. I just tried to run and got Cannot lint an empty CL
we also need to make sure we're using cpplint.py in all cases
There was a problem hiding this comment.
Did you have any modification in src/brave?
That command tries to reformat files that has changes.
There was a problem hiding this comment.
lint should fail right now whether there are changes or not because I've seen dozens of what would be lint errors in the code
There was a problem hiding this comment.
If you use format it will detect the errors.
As I told you in DM, if we want to use cpplint.py instead of clang_format, git cl would not be the good choice.
This command automatically formats a pending patch according to Chromium style.
simonhong
force-pushed
the
introduce_npm_lint
branch
from
49ff724
to
4301fcc
Compare
|
Will reopen again when ready. |
This command automatically formats a pending patch of brave core according to Chromium style.
See documents about
git cl format: https://chromium.googlesource.com/chromium/src/+/lkcr/docs/clang_format.mdclose #116
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
run
yarn lintafter making changes in brave-core.Reviewer Checklist: