- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.6k
build: introduce ci targets for lint/benchmark #5921
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
build: introduce ci targets for lint/benchmark #5921
Conversation
Introduce two new targets we will populate with actions once merged into all branches we need to support through CI.
        
          
                Makefile
              
                Outdated
          
        
      | bench-all bench bench-misc bench-array bench-buffer bench-net \ | ||
| bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \ | ||
| test-v8-benchmarks test-v8-all v8 | ||
| test-v8-benchmarks test-v8-all v8 lint-ci | 
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.
Can we remove lint at the beginning of this then?
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.
no, its still a target?
| Do we need to also reserve a  | 
| @mscdex said: 
 I was thinking no since we could just do what we need to in  | 
| LGTM then | 
| As long as we have  | 
| LGTM | 
| LGTM | 
    
      
        1 similar comment
      
    
  
    | LGTM | 
Introduce two new targets we will populate with actions once merged into all branches we need to support through CI. PR-URL: #5921 Reviewed-By: Brian White <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: João Reis <[email protected]> Reviewed-By: James M Snell <[email protected]>
| Landed in 8dcb82d | 
Introduce two new targets we will populate with actions once merged into all branches we need to support through CI. PR-URL: #5921 Reviewed-By: Brian White <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: João Reis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Introduce two new targets we will populate with actions once merged into all branches we need to support through CI. PR-URL: #5921 Reviewed-By: Brian White <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: João Reis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Pull Request check-list
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
Affected core subsystem(s)
build
Description of change
Introduce two new targets we will populate with actions once merged into all branches we need to support through CI. Refs #5638.
The gist of the problem is that we can't call makefile targets through CI that doesn't exist in all branches we test on. Meaning, the quicker we get it in, the easier it will be to expand on changes to linting, testing and benchmarking (making sure we call respective
-citargets from CI).vcbuild.batchanges are missing, but linting and benchmarking needs more improvements than just ci - and we don't invoke linting/benchmarking stuff from Windows just yet./cc @nodejs/build, @mscdex