code-and-learn icon indicating copy to clipboard operation
code-and-learn copied to clipboard

Common questions at C&L events

Open BridgeAR opened this issue 7 years ago • 7 comments

This issue is meant as a kind of FAQ for mentees and mentors.

Please add comments for things that you might encounter so others can look this up as well.

Questions:

  • How to make sure the code is covered without running make coverage.
    • A simple solution is to just add a console.log statement at the code point that should be reached and to check the console output.
  • How to get the test output from console.log() in the Node core tests?
    • Just run the code with ./node ./test/parallel/test-foo.js instead of python tools/test.py parallel/test-foo.js. If the test file has a "flags" comment at the top, start Node.js with those manually, e.g., ./node --expose-internals ./test/parallel/test-foo.js.
  • How to run a single test or all tests from a specific sub system?
    • Either use ./node relative-or-absolute-test-file-path.js or python tools/test.py subsystem/file-name.js for a single file or python tools/test.py -J subsystem for a whole subsystem or python tools/test.py -J glob
  • How to create a new test?
    • Check already existing tests. The test passes if the file executes without error. So just write the test as any other regular code.
  • What should be done with the pull request check marks if there is no documentation involved?
    • Please just mark the check mark as done or remove the entry if not applicable.
  • What should be done if make test shows failing tests on a local machine having the Nodejs master branch checked out without any changes?
    • Please check if there is an open issue that already summarizes the issue on your platform. If not, please open a new issue on the main Nodejs repository and copy the output in that issue, add all system information you have and under what circumstances the test(s) failed. If you are able to provide additional information to an existing issue, please do that as well.
  • The pull request got a "author ready" label but the code is not immediately merged. Why not?
    • We try to merge C&L PRs fast but for some PRs we'll want to make sure more collaborators got the chance to look at it. Therefore there are some rules in place how long a pull request has to be open before it may land. It also has to have a full CI run that has to be green so we are able to land it. So please be patient. If there is no activity for multiple days, just ask for reviews.
  • TBD...

BridgeAR avatar Oct 11 '18 23:10 BridgeAR

  1. [macOS] test-cluster-bind-privileged-port and test-cluster-shared-handle-bind-privileged-port fail on macOS Mojave, see https://github.com/nodejs/node/issues/21679. Solution: known Mojave issue, just ignore those two.
  2. [macOS] Tests that launch subprocesses and wait for them to abort (e.g. test-process-abort-exitcode and test-domain-with-abort-on-uncaught-exception) take long to run and/or timeout on macOS with CrashReporter enabled. Solution: disable CrashReporter, see https://github.com/nodejs/node/issues/13527#issuecomment-311672640.
  3. Quite often, people have (slightly?) outdated master branch versions. git pull upstream master =).
  4. [Windows]: There was a windows-specific problem with installing a proper Visual Studio build tools. Installing just "Visual C++ build tools" (as https://github.com/nodejs/node/blob/master/BUILDING.md#windows-1 recommenda) didn't help. Installing some more Visual Studio-related stuff helped, but I don't have the details — I wasn't the one who resolved that.

ChALkeR avatar Oct 12 '18 17:10 ChALkeR

Those issues are not code-and-learn specific, I think that they should be either resolved or mentioned in the documentation (e.g. BUILDING.md).

ChALkeR avatar Oct 12 '18 20:10 ChALkeR

Not necessarily 100% relevant but one comment I got was that its not always obvious who the mentors are. Something like wearing the same shirt or hats that stand out might help ?

So the question would be "who are the mentors"

mhdawson avatar Oct 16 '18 19:10 mhdawson

It would be useful if the tasks being assigned include the issue number of the associated issue so that committers can include a reference in their commit message.

mcqj avatar Nov 06 '18 16:11 mcqj

@mcqj Icreated these tasks specifically for the code and learn event and there are no issues for them. It would have been even more work to create issues and I hope you all had a good experience also without such a reference!

BridgeAR avatar Nov 06 '18 18:11 BridgeAR

No problem at all @BridgeAR

mcqj avatar Nov 15 '18 12:11 mcqj

  1. I completed my first PR, where do I go next? How do I get the list of next level PRs that I can make?

Please review https://www.nodetodo.org/next-steps/

  1. Did you make this (the code change requirement) on purpose, for the new comers?

No, these are incremental improvements that we identified but kept on stock for this exercise. We did not artifically created these issues for the purpose of code & learn.

  1. What happens if I raise PR on the master of my fork?

If you are raising a single PR no problem with that. When a second undertaking is being worked, the existing branch carries the first PR's content that is not desirable. So the best practise is to create one branch per PR and work inside that, for isolation and easy management.

  1. How to run single test?

Exaplained above.

  1. How do I know whether lint (or everything) passes? my output was too long, and at the end of it did not say success or failure.

If there is an error on any of the passes, the bottom of the log will clearly say so. If no error report found, that means all is well.

  1. I am new to git and github. Can you teach me that first?

Sure, will plan for that next time.

gireeshpunathil avatar Nov 24 '18 04:11 gireeshpunathil