git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v6 1/2] documentation: add tutorial for first contribution

From
Emily Shaffer <emilyshaffer@google.com>
Date
May 29, 2019, 20:09 UTC
Message-ID
<20190529200944.GA52337@google.com>
In-Reply-To
<CAP8UFD2YH50Br4BNmTqEVeUknxi1X39JCRB4XMwK8rx3DWx=KA@mail.gmail.com>
On Sun, May 26, 2019 at 09:48:26AM +0200, Christian Couder wrote:
Show 16 quoted lines
> On Fri, May 17, 2019 at 11:48 PM Emily Shaffer <emilyshaffer@google.com> 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.
> 
> Very nice, thanks! I tried it and I liked it very much.
> 
> I noted a few nits that might help improve it a bit.
> 
> > +----
> > +$ git clone https://github.com/git/git git
> 
> Nit: maybe add "$ cd git" after that.
Sure, done.
Show 12 quoted lines
> 
> > +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
> > +----
> 
> Nit: when building a "git-psuh" binary is created at the root of the
> repo which will pollute the `git status` output. The usual way we deal
> with that is by adding "/git-psuh" to the ".gitignore" at the root of
> the repo.

Right you are - good catch. I'll add a paragraph about adding to the gitignore.

Show 10 quoted lines
> 
> > +=== Implementation
> > +
> > +It's probably useful to do at least something besides printing out a string.
> > +Let's start by having a look at everything we get.
> > +
> > +Modify your `cmd_psuh` implementation to dump the args you're passed:
> 
> Nit: maybe it could be a bit more clear that the previous printf()
> call should be kept as is, otherwise the test added later will fail.
Done.
Show 9 quoted lines
> 
> > +----
> > +       const char *cfg_name;
> > +
> > +...
> > +
> > +       git_config(git_default_config, NULL)
> 
> Nit: a ";" is missing at the end of the above line.
Yikes, done.
Show 8 quoted lines
> 
> > +Let's commit this as well.
> > +
> > +----
> > +$ git commit -sm "psuh: print the current branch"
> 
> Nit: maybe add "builtin/psuh.c" at the end of the above line, so that
> a `git add builtin/psuh.c` is not needed.

This is purely personal preference, but I prefer manually adding files first. I didn't add any indication about staging the changes to psuh.c though, so I'm adding a line to `git add builtin/psuh.c`. I found one other place where the commit line was shown without the add line, so I included the add there too.

Show 30 quoted lines
> 
> > +....
> > +git-psuh(1)
> > +===========
> > +
> > +NAME
> > +----
> > +git-psuh - Delight users' typo with a shy horse
> > +
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'git-psuh'
> > +
> > +DESCRIPTION
> > +-----------
> > +...
> > +
> > +OPTIONS[[OPTIONS]]
> > +------------------
> > +...
> > +
> > +OUTPUT
> > +------
> > +...
> > +
> > +
> 
> Nit: it seems that the above newline could be removed.
Sure, why not.
> 
> Thanks,
> Christian.

Thanks for trying it out and for your thorough review, Christian. I appreciate it! Since this is checked into next already, I'll be sending a follow-on patch in reply to my last version in this thread which is based on the tip of next.

 - Emily
Previous: Christian CouderNext: SZEDER Gábor
Message 40 of 44 in “documentation: add lab for first contribution”
  1. 0/1 documentation: add lab for first contributionEmily Shaffer via GitGitGadget, Apr 11, 2019
  2. 1/1 documentation: add lab for first contributionEmily Shaffer via GitGitGadget, Apr 11, 2019
  3. Junio C HamanoApr 12, 2019
  4. Emily ShafferApr 12, 2019
  5. Junio C HamanoApr 13, 2019
  6. Emily ShafferApr 15, 2019
  7. Josh SteadmonApr 11, 2019
  8. Junio C HamanoApr 12, 2019
  9. Emily ShafferApr 12, 2019
  10. 0/1 documentation: add lab for first contributionEmily Shaffer via GitGitGadget, Apr 16, 2019
  11. 1/1 documentation: add lab for first contributionEmily Shaffer via GitGitGadget, Apr 16, 2019
  12. Junio C HamanoApr 17, 2019
  13. Eric SunshineApr 17, 2019
  14. Junio C HamanoApr 18, 2019
  15. Emily ShafferApr 17, 2019
  16. Emily ShafferApr 16, 2019
  17. documentation: add lab for first contributionEmily Shaffer, Apr 19, 2019
  18. Junio C HamanoApr 21, 2019
  19. Emily ShafferApr 22, 2019
  20. documentation: add tutorial for first contributionEmily Shaffer, Apr 23, 2019
  21. Josh SteadmonApr 30, 2019
  22. Emily ShafferMay 2, 2019
  23. Phil HordMay 3, 2019
  24. Emily ShafferMay 7, 2019
  25. Jonathan TanMay 6, 2019
  26. Emily ShafferMay 7, 2019
  27. Jonathan TanMay 7, 2019
  28. Junio C HamanoMay 8, 2019
  29. 0/2 documentation: add lab for first contributionEmily Shaffer, May 7, 2019
  30. 1/2 documentation: add tutorial for first contributionEmily Shaffer, May 7, 2019
  31. Emily ShafferMay 7, 2019
  32. Junio C HamanoMay 8, 2019
  33. Emily ShafferMay 8, 2019
  34. Jonathan TanMay 8, 2019
  35. 2/2 documentation: add anchors to MyFirstContributionEmily Shaffer, May 7, 2019
  36. Junio C HamanoMay 8, 2019
  37. 0/2 documentation: add tutorial for first contributionEmily Shaffer, May 17, 2019
  38. 1/2 documentation: add tutorial for first contributionEmily Shaffer, May 17, 2019
  39. Christian CouderMay 26, 2019
  40. Emily ShafferMay 29, 2019
  41. SZEDER GáborOct 18, 2019
  42. Emily ShafferOct 18, 2019
  43. 2/2 documentation: add anchors to MyFirstContributionEmily Shaffer, May 17, 2019
  44. doc: add some nit fixes to MyFirstContributionEmily Shaffer, May 29, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.