Skip to content

Instantly share code, notes, and snippets.

@chrisdickinson chrisdickinson/notes.md Secret
Last active Aug 29, 2015

Embed
What would you like to do?

onboarding

  • thank you for doing this

    • going to cover three things:
      • local setup
      • issues, labels, and reviewing code
      • merging code
  • setup:

    • CI lives here: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/
      • make sure to log in – I've sent your emails to rvagg, who will set up an account for you
      • go to "Build with parameters"
      • fill in user, repo, and branch. pr number is unused.
    • other repos:
    • notifications setup
      • END GOAL: if someone @-mentions you or your team, you should see it
      • best route I've found: unwatch the repo (I know this is silly)
      • or turn off email notifications for watching
        • otherwise it's hard to find things you care about in the email
      • YMMV
    • make sure you have whitespace=fix: git config --global --add core.whitespace fix
    • working dirs (clone the repo twice)
      • fork the repo to your user account on github
      • git clone git://github.com/iojs/io.js.git work-iojs
      • git clone work-iojs MERGE-iojs
      • cd work-iojs; git remote add chris git@github.com:chrisdickinson/io.js.git; git fetch;
      • cd ../MERGE-iojs; git remote remove origin; git remote add origin git@github.com:iojs/io.js.git; git fetch
    • #iojs on freenode is the best place to be to interact with the TC
  • issues + labels

    • you have free reign – don't hesitate to close an issue if you are confident that it should be closed

      • this will come more naturally over time
    • subsystems:

      • doc, tools, test, src (special)
      • lib/*.js
    • cc'ing humans (by subsystem):

      • lib/child_process: bnoordhuis, piscisaereus, cjihrig
      • lib/cluster: bnoordhuis
      • lib/net: indutny, bnoordhuis, piscisaereus, chrisdickinson, iojs/streams
      • lib/http: isaacs, bnoordhuis
      • lib/vm: domenic, isaac
      • lib/buffer: trevnorris
      • src/async-wrap.*: trevnorris
      • lib/crypto,tls,https: indutny
      • src/node_crypto.*: indutny
      • IN GENERAL: iojs/tc, or git shortlog -n -s <file>
    • labels:

      • tc-agenda if a topic seems controversial
      • major, minor, patch:
        • be conservative – that is, if it has the chance of breaking something, go for major

        • when adding the label, please add a comment explaining why you're adding it

          • it's cached locally in your brain at that moment!
        • minor vs. patch: roughly: "does it add a new method / does it add a new section to the docs"

        • major vs. everything else: run last versions tests against this version, if they pass, minor or patch

        • adding affected subsystem, os, + iojs version labels is good as well!

        • https://gist.github.com/chrisdickinson/ba532fa0e4e243fb7b44 – test commits

          git checkout $(git show -s --pretty='%T' $(git show-ref -d $(git describe --abbrev=0) | tail -n1 | awk '{print $1}')) -- test; make test

    • reviewing:

      • primary goal is for the codebase to improve
      • secondary (but not far off) is for the person submitting code to succeed
        • helps grow the community
        • and draws new people into the project
      • be aware: your opinion carries a lot of weight!
      • nits are fine, but try to avoid stalling the PR
        • note that they are nits when you comment
        • if they really are nits, feel free to fix them yourself on merge (especially for new contributors)
        • improvement doesn't have to come all at once
      • set time limits and expectations:
        • before merging code: give folks at least one working day to respond: "If no one objects, tomorrow at I'll merge this in."
          • set reminders
        • check in on the code every once in a while (set reminders!)
        • if a PR is abandoned, check if they'd mind if you took it over (especially if its just nits left)
      • what belongs in iojs:
        • opinions vary, but I find the following helpful:
        • if io.js itself needs it, then it belongs in io.js
          • that is to say, url is there because of http, freelist is there because of http, et al
  • process for getting code in:

    • the collaborator guide is a great resource: https://github.com/iojs/io.js/blob/v1.x/COLLABORATOR_GUIDE.md#technical-howto
    • no one (not even TC) pushes directly to master without review
      • best practice: work in WORK-iojs, commit often, open a PR
      • make sure to spend time on the description: every moment you spend writing a good description halves the amount of time it takes to understand your code.
      • only squash at the end of your work
      • one "LGTM" is sufficient
      • you have the power to LGTM another collaborator or TC members' work
      • make sure to run the PR through CI first!
    • once code is ready to go in:
      • cd MERGE-iojs
      • git status; git pull origin v1.x
      • curl -sL 'url-of-pr.patch' | less
      • if it all looks good, curl -sL 'url-of-pr.patch' | git am
      • git rebase -i origin/v1.x
      • squash into logical commits
      • make clean; ./configure; make -j8; make test (-j8 builds iojs in parallel with 8 cores. adjust to the number of cores your processor has for best results.)
      • commits should follow subsystem[,subsystem]: small description\n\nbig description\n\n<metadata>
      • first line 50, all others 72
      • add metadata:
        • Fixes:
        • Reviewed-By: human
        • PR-URL:
      • make clean; make test-ci; git push origin v1.x
      • close the original PR with "merged in <commit hash>".
    • what if something goes wrong?
      • ping a TC member
      • if git am fails – use git am --abort
        • this usually means the PR needs updated
        • prefer to make the originating user update the code, since they have it fresh in mind
        • if you must, git fetch origin pull/N/head:pr-N; git checkout pr-N; git rebase v1.x
  • exercise: make PRs adding yourselves to the README.

  • final note:

    • don't worry about making mistakes: everybody makes them, there's a lot to internalize and that takes time (& we recognize that!)
    • very few (no?) mistakes are unrecoverable
    • the existing io.js committers trust you and are grateful for your help!

https://docs.google.com/forms/d/1wtwhnabArplRSEkerkx9JPA1y3F-aWzGWtFFlL3Fwdg/viewform?usp=send_form

@Fishrock123

This comment has been minimized.

Copy link

commented Jan 23, 2015

best to clone the repo twice (I am paranoid)

I think it would be best to show a way of not being paranoid of git, probably better for everyone in the long run. :)

@Fishrock123

This comment has been minimized.

Copy link

commented Jan 23, 2015

iojs

io.js -- probably best to include some info in chat if someone is not as familiar with irc

@chrisdickinson

This comment has been minimized.

Copy link
Owner Author

commented Jan 24, 2015

@Fishrock123 I'm not paranoid of git, I'm paranoid of me :) I prefer to set things up so that making a mistake isn't possible, because as a human I am prone to making them if they're available.

@bnoordhuis

This comment has been minimized.

Copy link

commented Jan 24, 2015

if you need help setting up an irc bouncer

There's also http://logs.libuv.org/io.js/latest, that's what I use to stay informed.

one "LGTM" or +1 is sufficient

I prefer LGTM. +1 is somewhat ambiguous in that it's unclear whether the commenter reviewed the PR in depth or just thinks that the general thrust is good.

@mscdex

This comment has been minimized.

Copy link

commented Mar 15, 2015

Are there instructions on how to start a job on jenkins for a PR? I must have missed that somewhere...

@silverwind

This comment has been minimized.

Copy link

commented Apr 4, 2015

Some might find this helpful for the local setup:

git clone git://github.com/iojs/io.js.git iojs
git clone iojs iojs-merge
cd iojs
git remote add silverwind git@github.com:silverwind/io.js.git
git remote set-url --push origin no_push
git fetch
cd ../iojs-merge
git remote remove origin
git remote add origin git@github.com:iojs/io.js.git
git fetch

Replace my remote with yours. Result will be ./iojs and ./iojs-merge. Pushing to origin is disabled on the former repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.