-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 a prePR command and PR template to remind people to run it #2583
Conversation
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.
Looks good :)
Codecov Report
@@ Coverage Diff @@
## master #2583 +/- ##
=======================================
Coverage 95.14% 95.14%
=======================================
Files 361 361
Lines 6634 6634
Branches 294 294
=======================================
Hits 6312 6312
Misses 322 322 Continue to review full report at Codecov.
|
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.
👍
@@ -697,6 +697,8 @@ addCommandAlias("validateKernelJS", "kernelLawsJS/test") | |||
addCommandAlias("validateFreeJS", "freeJS/test") //separated due to memory constraint on travis | |||
addCommandAlias("validate", ";clean;validateJS;validateKernelJS;validateFreeJS;validateJVM") | |||
|
|||
addCommandAlias("prePR", ";fmt;validateBC") |
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.
I think that this is fine for now, but in the near future I'd like us to get to a branching setup where binary-incompatible changes are okay for the right branches or something.
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.
@ceedubs yes. It's probably possible that validateBC would just mean different thing on different branch or something.
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.
I made a minor wording suggestion, but I'm also fine with this being merged in its current form.
Co-Authored-By: kailuowang <[email protected]>
73c6f21
No description provided.