-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
doc: recompile when testing in CONTRIBUTING.md #2051
Conversation
👍 Maybe you could also fix the command right before from |
+1 to @targos's suggestion. Also, it may be better to just have |
Nice one @targos, I'll fix it shortly. 👍 for |
I missed something in my suggestion, there is a typo in the file name. |
@phillipj not too much, no. Also |
cc31ed1
to
25be43e
Compare
Somewhat concerned about advocating maximum parallelism with make as it made my machine come to a grinding halt. |
9d21135
to
628a3ab
Compare
Hmmm, I've never actually just run I'm in favor of just suggesting |
25be43e
to
4ee6be4
Compare
Alright, that should prevent devs getting load of 300 and a non-responding OS as the plain |
Changes to core modules does not take effect before project gets recompiled. Tip new contributors about this when describing how to to run tests in contribution guide. Removed `jslint` from first test command example as jslint are included when running `make test`. Fixed wrong path of example stream2-transform test. PR-URL: nodejs#2051
4ee6be4
to
b8789d9
Compare
I just added PR-URL to the commit. |
LGTM. Is there a reason this hasn't been merged yet? |
Not really |
LGTM too. |
Just say the word if there's anything I should do to get this landed. |
Changes to core modules do not take effect unless recompiled. Tip new contributors about this when describing how to run tests in contribution guide. Removed `jslint` from first test command example, as jslint is included when running `make test`. Fixed wrong path of example stream2-transform test. PR-URL: #2051 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
👍 and thanks :) |
@thefourtheye is there something wrong with your system time? c7d8b09 was committed in the future (today, 5pm UTC) |
it could also be the reason why your commit wasn't automatically taken for the 2.4.0 changelog |
@targos Oh, I am not sure what is wrong. I resynced my machine's time now. Let's see if this happens again. But that commit was landed after 2.4.0 build started, right? |
@thefourtheye I don't think so, it landed before mine. |
Changes to core modules does not take effect before project gets recompiled. This was not obvious to me when trying to run some tests at first, which made me bother you with an issue a few weeks ago.
Added a line tipping new contributors about this behavior when describing how to to run tests in contribution guide.
Removed
jslint
from first test command example as jslint are included when runningmake test
.Fixed wrong path of example stream2-transform test.