On Sun, May 26, 2019 at 09:48:26AM +0200, Christian Couder wrote:

On Fri, May 17, 2019 at 11:48 PM Emily Shaffer [off-list ref] wrote:

quoted

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.

quoted

+----
+$ git clone https://github.com/git/git git

Nit: maybe add "$ cd git" after that.

Sure, done.

quoted

+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.

quoted

+=== 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.

quoted

+----
+       const char *cfg_name;
+
+...
+
+       git_config(git_default_config, NULL)

Nit: a ";" is missing at the end of the above line.

Yikes, done.

quoted

+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.

quoted

+....
+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