On Thu, May 02, 2019 at 07:11:04PM -0700, Phil Hord wrote: > On Tue, Apr 23, 2019 at 12:35 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote: > > > > This tutorial covers how to add a new command to Git and, in the > > process, everything from cloning git/git to getting reviewed on the > > mailing list. It's meant for new contributors to go through > > interactively, learning the techniques generally used by the git/git > > development community. > > > > Thanks for working on this. It's very nicely done. :) > > +Check it out! You've got a command! Nice work! Let's commit this. > > + > > +---- > > +$ git add Makefile builtin.h builtin/psuh.c git.c > > +$ git commit -s > > +---- > > + > > +You will be presented with your editor in order to write a commit message. Start > > +the commit with a 50-column or less subject line, including the name of the > > +component you're working on. Remember to be explicit and provide the "Why" of > > This part sounds a little ambiguous to me, as I'm expected to include > the "Why" in my 50-column subject line. I don't want to go overboard, > but maybe direct them further to > > After this, insert a blank line (always required) and then some > text describing > your change. Remember to be explicit and ... > Done. I agree it's ambiguous about how much is supposed to go into the subject versus the body, so I've hopefully clarified by explaining that the body "should provide the bulk of the context". > > +---- > > +$ make all doc > > +$ man Documentation/git-psuh.1 > > +---- > > + > > +or > > + > > +---- > > +$ make -C Documentation/git-psuh.1 > > There's an unwanted slash here. This should be `make -C Documentation > git-psuh.1`. Done, thanks. Nice catch. > > +=== Overview of Testing Structure > > + > > +The tests in Git live in `t/` and are named with a 4-decimal digit, according to > > This doesn't parse. How about this? > > named with a 4-decimal digit number using the schema shown in ... > I think we've both managed to miss that I've swapped "decimal" and "digit" by accident. :) How about "named with a 4-digit decimal number using the schema"? Very pleased that you caught this, since all the once-overs in the world from the cooked-brain author and the cooked-brain second- or tenth-pass reviewers wouldn't have caught this mistake. Thanks! > > +== Sending Patches via GitGitGadget > > + > > +One option for sending patches is to follow a typical pull request workflow and > > +send your patches out via GitGitGadget. GitGitGadget is a tool created by > > +Johannes Schindelin to make life as a Git contributor easier for those used to > > +the GitHub PR workflow. It allows contributors to open pull requests against its > > +mirror of the Git project, and does some magic to turn the PR into a set of > > +emails and sent them out for you. It also runs the Git continuous integration > > nit: "send" them out for you. Done, thanks. > > > +suite for you. It's documented at http://gitgitgadget.github.io. > > + > > +=== Forking git/git on GitHub > > + > > +Before you can send your patch off to be reviewed using GitGitGadget, you will > > +need to fork the Git project and upload your changes. First thing - make sure > > +you have a GitHub account. > > + > > +Head to the https://github.com/git/git[GitHub mirror] and look for the Fork > > +button. Place your fork wherever you deem appropriate and create it. > > + > > +=== Uploading To Your Own Fork > > I noticed some of your titles Use Capital Initials and others do not. > I suppose either is fine, but consistency is appreciated. > Nice catch. I've gone through and fixed up the titles throughout; as a result I also caught a missed monospace. > > +=== Sending Your Patches > > + > > +Now that your CI is passing and someone has granted you permission to use > > +GitGitGadget with the `/allow` command, sending out for review is as simple as > > nit: extra space before "sending" > Done. > > +Next, go look at your pull request against GitGitGadget; you should see the CI > > +has been kicked off again. Now while the CI is running is a good time for you > > nit: extra spaces before "kicked" > Done. > > +Make sure you retain the ``[PATCH 0/X]'' part; that's what indicates to the Git > > +community that this email is the beginning of a review, and many reviewers > > +filter their email for this type of flag. > > + > > +You'll need to add some extra > > Early line break on this line. > Done. > > +Now send the emails again, paying close attention to which messages you pass in > > +to the command: > > + > > +---- > > +$ git send-email --to=target@xxxxxxxxxxx > > + --in-reply-to=<foo.12345.author@xxxxxxxxxxx> > > You probably need quotes around this message-id argument to avoid the > shell interpreting it as redirection. > Indeed. And it looks like I also missed, here and above, specifying the actual patch files to mail. Whoops... :) > > +---- > > + > > +=== Bonus Chapter: One-Patch Changes > > + > > +In some cases, your very small change may consist of only one patch. When that > > +happens, you only need to send one email. Your commit message should already be > > +meaningful and explain the at a high level the purpose (what is happening and > > typo: "explain at a high level" > Done. > > +If the topic has already been merged to `next`, rather than modifying your > > +patches with `git rebase -i`, you should make further changes incrementally - > > +that is, with another commit, based on top of of the maintainer's topic branch > > typo: "of of" > Done. > > +as detailed in https://github.com/gitster/git. Your work is still in the same > > +topic but is now incremental, rather than a wholesale rewrite of the topic > > +branch. > > + > > +The topic branches in the maintainer's GitHub are mirrored in GitGitGadget, so > > +if you're sending your reviews out that way, you should be sure to open your PR > > +against the appropriate GitGitGadget/Git branch. > > + > > +If you're using `git > > +send-email`, you can use it the same way as before, but you should generate your > > Early line break on this line inside the `git send-email` command. > Done. Thanks very much for the thorough review, Phil! I appreciate it!
