git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phil Hord <phil.hord@gmail.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: Git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v4] documentation: add tutorial for first contribution
Date: Thu, 2 May 2019 19:11:04 -0700	[thread overview]
Message-ID: <CABURp0rE23SCxB4VD0-kVWp6OfS7-4O6biyD7zMqSUQvR_RZxg@mail.gmail.com> (raw)
In-Reply-To: <20190423193410.101803-1-emilyshaffer@google.com>

On Tue, Apr 23, 2019 at 12:35 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.
>

Thanks for working on this.  It's very nicely done.

> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> Only minor changes from v3, correcting the comments Junio made in his
> review.
>
> - Changed commit subject
> - Stray monospace typos
> - Curly brace style
>
>  Documentation/Makefile                |    1 +
>  Documentation/MyFirstContribution.txt | 1073 +++++++++++++++++++++++++
>  2 files changed, 1074 insertions(+)
>  create mode 100644 Documentation/MyFirstContribution.txt
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 26a2342bea..fddc3c3c95 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -74,6 +74,7 @@ API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technica
>  SP_ARTICLES += $(API_DOCS)
>
>  TECH_DOCS += SubmittingPatches
> +TECH_DOCS += MyFirstContribution
>  TECH_DOCS += technical/hash-function-transition
>  TECH_DOCS += technical/http-protocol
>  TECH_DOCS += technical/index-format
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> new file mode 100644
> index 0000000000..fc4a59a8c6
> --- /dev/null
> +++ b/Documentation/MyFirstContribution.txt
> @@ -0,0 +1,1073 @@
> +My First Contribution to the Git Project
> +========================================
> +
> +== Summary
> +
> +This is a tutorial demonstrating the end-to-end workflow of creating a change to
> +the Git tree, sending it for review, and making changes based on comments.
> +
> +=== Prerequisites
> +
> +This tutorial assumes you're already fairly familiar with using Git to manage
> +source code.  The Git workflow steps will largely remain unexplained.
> +
> +=== Related Reading
> +
> +This tutorial aims to summarize the following documents, but the reader may find
> +useful additional context:
> +
> +- `Documentation/SubmittingPatches`
> +- `Documentation/howto/new-command.txt`
> +
> +== Getting Started
> +
> +=== Pull the Git codebase
> +
> +Git is mirrored in a number of locations. https://git-scm.com/downloads
> +suggests one of the best places to clone from is GitHub.
> +
> +----
> +$ git clone https://github.com/git/git git
> +----
> +
> +=== Identify Problem to Solve
> +
> +////
> +Use + to indicate fixed-width here; couldn't get ` to work nicely with the
> +quotes around "Pony Saying 'Um, Hello'".
> +////
> +In this tutorial, we will add a new command, +git psuh+, short for ``Pony Saying
> +`Um, Hello''' - a feature which has gone unimplemented despite a high frequency
> +of invocation during users' typical daily workflow.
> +
> +(We've seen some other effort in this space with the implementation of popular
> +commands such as `sl`.)
> +
> +=== Set Up Your Workspace
> +
> +Let's start by making a development branch to work on our changes. Per
> +`Documentation/SubmittingPatches`, since a brand new command is a new feature,
> +it's fine to base your work on `master`. However, in the future for bugfixes,
> +etc., you should check that document and base it on the appropriate branch.
> +
> +For the purposes of this document, we will base all our work on the `master`
> +branch of the upstream project. Create the `psuh` branch you will use for
> +development like so:
> +
> +----
> +$ git checkout -b psuh origin/master
> +----
> +
> +We'll make a number of commits here in order to demonstrate how to send a topic
> +with multiple patches up for review simultaneously.
> +
> +== Code It Up!
> +
> +NOTE: A reference implementation can be found at
> +https://github.com/nasamuffin/git/tree/psuh.
> +
> +=== Adding a new command
> +
> +Lots of the subcommands are written as builtins, which means they are
> +implemented in C and compiled into the main `git` executable. Implementing the
> +very simple `psuh` command as a built-in will demonstrate the structure of the
> +codebase, the internal API, and the process of working together as a contributor
> +with the reviewers and maintainer to integrate this change into the system.
> +
> +Built-in subcommands are typically implemented in a function named "cmd_"
> +followed by the name of the subcommand, in a source file named after the
> +subcommand and contained within `builtin/`. So it makes sense to implement your
> +command in `builtin/psuh.c`. Create that file, and within it, write the entry
> +point for your command in a function matching the style and signature:
> +
> +----
> +int cmd_psuh(int argc, const char **argv, const char *prefix)
> +----
> +
> +We'll also need to add the extern declaration of psuh; open up `builtin.h`,
> +find the declaration for `cmd_push`, and add a new line for `psuh` immediately
> +before it, in order to keep the declarations sorted:
> +
> +----
> +extern int cmd_psuh(int argc, const char **argv, const char *prefix);
> +----
> +
> +Be sure to `#include "builtin.h"` in your `psuh.c`.
> +
> +Go ahead and add some throwaway printf to that function. This is a decent
> +starting point as we can now add build rules and register the command.
> +
> +NOTE: Your throwaway text, as well as much of the text you will be adding over
> +the course of this tutorial, is user-facing. That means it needs to be
> +localizable. Take a look at `po/README` under "Marking strings for translation".
> +Throughout the tutorial, we will mark strings for translation as necessary; you
> +should also do so when writing your user-facing commands in the future.
> +
> +----
> +int cmd_psuh(int argc, const char **argv, const char *prefix)
> +{
> +       printf(_("Pony saying hello goes here.\n"));
> +       return 0;
> +}
> +----
> +
> +Let's try to build it.  Open `Makefile`, find where `builtin/push.o` is added
> +to `BUILTIN_OBJS`, and add `builtin/psuh.o` in the same way next to it in
> +alphabetical order. Once you've done so, move to the top-level directory and
> +build simply with `make`. Also add the `DEVELOPER=1` variable to turn on
> +some additional warnings:
> +
> +----
> +$ echo DEVELOPER=1 >config.mak
> +$ make
> +----
> +
> +NOTE: When you are developing the Git project, it's preferred that you use the
> +`DEVELOPER` flag; if there's some reason it doesn't work for you, you can turn
> +it off, but it's a good idea to mention the problem to the mailing list.
> +
> +NOTE: The Git build is parallelizable. `-j#` is not included above but you can
> +use it as you prefer, here and elsewhere.
> +
> +Great, now your new command builds happily on its own. But nobody invokes it.
> +Let's change that.
> +
> +The list of commands lives in `git.c`. We can register a new command by adding
> +a `cmd_struct` to the `commands[]` array. `struct cmd_struct` takes a string
> +with the command name, a function pointer to the command implementation, and a
> +setup option flag. For now, let's keep cheating off of `push`. Find the line
> +where `cmd_push` is registered, copy it, and modify it for `cmd_psuh`, placing
> +the new line in alphabetical order.
> +
> +The options are documented in `builtin.h` under "Adding a new built-in." Since
> +we hope to print some data about the user's current workspace context later,
> +we need a Git directory, so choose `RUN_SETUP` as your only option.
> +
> +Go ahead and build again. You should see a clean build, so let's kick the tires
> +and see if it works. There's a binary you can use to test with in the
> +`bin-wrappers` directory.
> +
> +----
> +$ ./bin-wrappers/git psuh
> +----
> +
> +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 ...

> +your change, especially if it couldn't easily be understood from your diff. When
> +editing your commit message, don't remove the Signed-off-by line which was added
> +by `-s` above.
> +
> +----
> +psuh: add a built-in by popular demand
> +
> +Internal metrics indicate this is a command many users expect to be
> +present. So here's an implementation to help drive customer
> +satisfaction and engagement: a pony which doubtfully greets the user,
> +or, a Pony Saying "Um, Hello" (PSUH).
> +
> +This commit message is intentionally formatted to 72 columns per line,
> +starts with a single line as "commit message subject" that is written as
> +if to command the codebase to do something (add this, teach a command
> +that). The body of the message is designed to add information about the
> +commit that is not readily deduced from reading the associated diff,
> +such as answering the question "why?".
> +
> +Signed-off-by: A U Thor <author@example.com>
> +----
> +
> +Go ahead and inspect your new commit with `git show`. "psuh:" indicates you
> +have modified mainly the `psuh` command. The subject line gives readers an idea
> +of what you've changed. The sign-off line (`-s`) indicates that you agree to
> +the Developer's Certificate of Origin 1.1 (see the
> +`Documentation/SubmittingPatches` +++[[dco]]+++ header). If you wish to add some
> +context to your change, go ahead with `git commit --amend`.
> +
> +For the remainder of the tutorial, the subject line only will be listed for the
> +sake of brevity. However, fully-fleshed example commit messages are available
> +on the reference implementation linked at the top of this document.
> +
> +=== 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:
> +
> +----
> +       int i;
> +
> +       ...
> +
> +       printf(Q_("Your args (there is %d):\n",
> +                 "Your args (there are %d):\n",
> +                 argc),
> +              argc);
> +       for (i = 0; i < argc; i++) {
> +               printf("%d: %s\n", i, argv[i]);
> +       }
> +       printf(_("Your current working directory:\n<top-level>%s%s\n"),
> +              prefix ? "/" : "", prefix ? prefix : "");
> +
> +----
> +
> +Build and try it. As you may expect, there's pretty much just whatever we give
> +on the command line, including the name of our command. (If `prefix` is empty
> +for you, try `cd Documentation/ && ../bin-wrappers/git/ psuh`). That's not so
> +helpful. So what other context can we get?
> +
> +Add a line to `#include "config.h"`. Then, add the following bits to the
> +function body:
> +
> +----
> +       const char *cfg_name;
> +
> +...
> +
> +       git_config(git_default_config, NULL)
> +       if (git_config_get_string_const("user.name", &cfg_name) > 0) {
> +               printf(_("No name is found in config\n"));
> +       } else {
> +               printf(_("Your name: %s\n"), cfg_name);
> +       }
> +----
> +
> +`git_config()` will grab the configuration from config files known to Git and
> +apply standard precedence rules. `git_config_get_string_const()` will look up
> +a specific key ("user.name") and give you the value. There are a number of
> +single-key lookup functions like this one; you can see them all (and more info
> +about how to use `git_config()`) in `Documentation/technical/api-config.txt`.
> +
> +You should see that the name printed matches the one you see when you run:
> +
> +----
> +$ git config --get user.name
> +----
> +
> +Great! Now we know how to check for values in the Git config. Let's commit this
> +too, so we don't lose our progress.
> +
> +----
> +$ git add builtin/psuh.c
> +$ git commit -sm "psuh: show parameters & config opts"
> +----
> +
> +NOTE: Again, the above is for sake of brevity in this tutorial. In a real change
> +you should not use `-m` but instead use the editor to write a meaningful
> +message.
> +
> +Still, it'd be nice to know what the user's working context is like. Let's see
> +if we can print the name of the user's current branch. We can cheat off of the
> +`git status` implementation; the printer is located in `wt-status.c` and we can
> +see that the branch is held in a `struct wt_status`.
> +
> +`wt_status_print()` gets invoked by `cmd_status()` in `builtin/commit.c`.
> +Looking at that implementation we see the status config being populated like so:
> +
> +----
> +status_init_config(&s, git_status_config);
> +----
> +
> +But as we drill down, we can find that `status_init_config()` wraps a call
> +to `git_config()`. Let's modify the code we wrote in the previous commit.
> +
> +Be sure to include the header to allow you to use `struct wt_status`:
> +----
> +#include "wt-status.h"
> +----
> +
> +Then modify your `cmd_psuh` implementation to declare your `struct wt_status`,
> +prepare it, and print its contents:
> +
> +----
> +       struct wt_status status;
> +
> +...
> +
> +       wt_status_prepare(the_repository, &status);
> +       git_config(git_default_config, &status);
> +
> +...
> +
> +       printf(_("Your current branch: %s\n"), status.branch);
> +----
> +
> +Run it again. Check it out - here's the (verbose) name of your current branch!
> +
> +Let's commit this as well.
> +
> +----
> +$ git commit -sm "psuh: print the current branch"
> +----
> +
> +Now let's see if we can get some info about a specific commit.
> +
> +Luckily, there are some helpers for us here. `commit.h` has a function called
> +`lookup_commit_reference_by_name` to which we can simply provide a hardcoded
> +string; `pretty.h` has an extremely handy `pp_commit_easy()` call which doesn't
> +require a full format object to be passed.
> +
> +Add the following includes:
> +
> +----
> +#include "commit.h"
> +#include "pretty.h"
> +----
> +
> +Then, add the following lines within your implementation of `cmd_psuh()` near
> +the declarations and the logic, respectively.
> +
> +----
> +       struct commit *c = NULL;
> +       struct strbuf commitline = STRBUF_INIT;
> +
> +...
> +
> +       c = lookup_commit_reference_by_name("origin/master");
> +
> +       if (c != NULL) {
> +               pp_commit_easy(CMIT_FMT_ONELINE, c, &commitline);
> +               printf(_("Current commit: %s\n"), commitline.buf);
> +       }
> +----
> +
> +The `struct strbuf` provides some safety belts to your basic `char*`, one of
> +which is a length member to prevent buffer overruns. It needs to be initialized
> +nicely with `STRBUF_INIT`. Keep it in mind when you need to pass around `char*`.
> +
> +`lookup_commit_reference_by_name` resolves the name you pass it, so you can play
> +with the value there and see what kind of things you can come up with.
> +
> +`pp_commit_easy` is a convenience wrapper in `pretty.h` that takes a single
> +format enum shorthand, rather than an entire format struct. It then
> +pretty-prints the commit according to that shorthand. These are similar to the
> +formats available with `--pretty=FOO` in many Git commands.
> +
> +Build it and run, and if you're using the same name in the example, you should
> +see the subject line of the most recent commit in `origin/master` that you know
> +about. Neat! Let's commit that as well.
> +
> +----
> +$ git commit -sm "psuh: display the top of origin/master"
> +----
> +
> +=== Adding documentation
> +
> +Awesome! You've got a fantastic new command that you're ready to share with the
> +community. But hang on just a minute - this isn't very user-friendly. Run the
> +following:
> +
> +----
> +$ ./bin-wrappers/git help psuh
> +----
> +
> +Your new command is undocumented! Let's fix that.
> +
> +Take a look at `Documentation/git-*.txt`. These are the manpages for the
> +subcommands that Git knows about. You can open these up and take a look to get
> +acquainted with the format, but then go ahead and make a new file
> +`Documentation/git-psuh.txt`. Like with most of the documentation in the Git
> +project, help pages are written with AsciiDoc (see CodingGuidelines, "Writing
> +Documentation" section). Use the following template to fill out your own
> +manpage:
> +
> +// Surprisingly difficult to embed AsciiDoc source within AsciiDoc.
> +[listing]
> +....
> +git-psuh(1)
> +===========
> +
> +NAME
> +----
> +git-psuh - Delight users' typo with a shy horse
> +
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git-psuh'
> +
> +DESCRIPTION
> +-----------
> +...
> +
> +OPTIONS[[OPTIONS]]
> +------------------
> +...
> +
> +OUTPUT
> +------
> +...
> +
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
> +....
> +
> +The most important pieces of this to note are the file header, underlined by =,
> +the NAME section, and the SYNOPSIS, which would normally contain the grammar if
> +your command took arguments. Try to use well-established manpage headers so your
> +documentation is consistent with other Git and UNIX manpages; this makes life
> +easier for your user, who can skip to the section they know contains the
> +information they need.
> +
> +Now that you've written your manpage, you'll need to build it explicitly. We
> +convert your AsciiDoc to troff which is man-readable like so:
> +
> +----
> +$ 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`.

> +$ man Documentation/git-psuh.1
> +----
> +
> +NOTE: You may need to install the package `asciidoc` to get this to work.
> +
> +While this isn't as satisfying as running through `git help`, you can at least
> +check that your help page looks right.
> +
> +You can also check that the documentation coverage is good (that is, the project
> +sees that your command has been implemented as well as documented) by running
> +`make check-docs` from the top-level.
> +
> +Go ahead and commit your new documentation change.
> +
> +=== Adding usage text
> +
> +Try and run `./bin-wrappers/git psuh -h`. Your command should crash at the end.
> +That's because `-h` is a special case which your command should handle by
> +printing usage.
> +
> +Take a look at `Documentation/technical/api-parse-options.txt`. This is a handy
> +tool for pulling out options you need to be able to handle, and it takes a
> +usage string.
> +
> +In order to use it, we'll need to prepare a NULL-terminated usage string and a
> +`builtin_psuh_options` array. Add a line to `#include "parse-options.h"`.
> +
> +At global scope, add your usage:
> +
> +----
> +static const char * const psuh_usage[] = {
> +       N_("git psuh"),
> +       NULL,
> +};
> +----
> +
> +Then, within your `cmd_psuh()` implementation, we can declare and populate our
> +`option` struct. Ours is pretty boring but you can add more to it if you want to
> +explore `parse_options()` in more detail:
> +
> +----
> +       struct option options[] = {
> +               OPT_END()
> +       };
> +----
> +
> +Finally, before you print your args and prefix, add the call to
> +`parse-options()`:
> +
> +----
> +       argc = parse_options(argc, argv, prefix, options, psuh_usage, 0);
> +----
> +
> +This call will modify your `argv` parameter. It will strip the options you
> +specified in `options` from `argv` and the locations pointed to from `options`
> +entries will be updated. Be sure to replace your `argc` with the result from
> +`parse_options()`, or you will be confused if you try to parse `argv` later.
> +
> +It's worth noting the special argument `--`. As you may be aware, many Unix
> +commands use `--` to indicate "end of named parameters" - all parameters after
> +the `--` are interpreted merely as positional arguments. (This can be handy if
> +you want to pass as a parameter something which would usually be interpreted as
> +a flag.) `parse_options()` will terminate parsing when it reaches `--` and give
> +you the rest of the options afterwards, untouched.
> +
> +Build again. Now, when you run with `-h`, you should see your usage printed and
> +your command terminated before anything else interesting happens. Great!
> +
> +Go ahead and commit this one, too.
> +
> +== Testing
> +
> +It's important to test your code - even for a little toy command like this one.
> +Moreover, your patch won't be accepted into the Git tree without tests. Your
> +tests should:
> +
> +* Illustrate the current behavior of the feature
> +* Prove the current behavior matches the expected behavior
> +* Ensure the externally-visible behavior isn't broken in later changes
> +
> +So let's write some tests.
> +
> +Related reading: `t/README`
> +
> +=== 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 ...

> +the schema shown in the Naming Tests section of `t/README`.
> +
> +=== Writing Your Test
> +
> +Since this a toy command, let's go ahead and name the test with t9999. However,
> +as many of the family/subcmd combinations are full, best practice seems to be
> +to find a command close enough to the one you've added and share its naming
> +space.
> +
> +Create a new file `t/t9999-psuh-tutorial.sh`. Begin with the header as so (see
> +"Writing Tests" and "Source 'test-lib.sh'" in `t/README`):
> +
> +----
> +#!/bin/sh
> +
> +test_description='git-psuh test
> +
> +This test runs git-psuh and makes sure it does not crash.'
> +
> +. ./test-lib.sh
> +----
> +
> +Tests are framed inside of a `test_expect_success` in order to output TAP
> +formatted results. Let's make sure that `git psuh` doesn't exit poorly and does
> +mention the right animal somewhere:
> +
> +----
> +test_expect_success 'runs correctly with no args and good output' '
> +       git psuh >actual &&
> +       test_i18ngrep Pony actual
> +'
> +----
> +
> +Indicate that you've run everything you wanted by adding the following at the
> +bottom of your script:
> +
> +----
> +test_done
> +----
> +
> +Make sure you mark your test script executable:
> +
> +----
> +$ chmod +x t/t9999-psuh-tutorial.sh
> +----
> +
> +You can get an idea of whether you created your new test script successfully
> +by running `make -C t test-lint`, which will check for things like test number
> +uniqueness, executable bit, and so on.
> +
> +=== Running Locally
> +
> +Let's try and run locally:
> +
> +----
> +$ make
> +$ cd t/ && prove t9999-psuh-tutorial.sh
> +----
> +
> +You can run the full test suite and ensure `git-psuh` didn't break anything:
> +
> +----
> +$ cd t/
> +$ prove -j$(nproc) --shuffle t[0-9]*.sh
> +----
> +
> +NOTE: You can also do this with `make test` or use any testing harness which can
> +speak TAP. `prove` can run concurrently. `shuffle` randomizes the order the
> +tests are run in, which makes them resilient against unwanted inter-test
> +dependencies. `prove` also makes the output nicer.
> +
> +Go ahead and commit this change, as well.
> +
> +== Getting Ready to Share
> +
> +You may have noticed already that the Git project performs its code reviews via
> +emailed patches, which are then applied by the maintainer when they are ready
> +and approved by the community. The Git project does not accept patches from
> +pull requests, and the patches emailed for review need to be formatted a
> +specific way. At this point the tutorial diverges, in order to demonstrate two
> +different methods of formatting your patchset and getting it reviewed.
> +
> +The first method to be covered is GitGitGadget, which is useful for those
> +already familiar with GitHub's common pull request workflow. This method
> +requires a GitHub account.
> +
> +The second method to be covered is `git send-email`, which can give slightly
> +more fine-grained control over the emails to be sent. This method requires some
> +setup which can change depending on your system and will not be covered in this
> +tutorial.
> +
> +Regardless of which method you choose, your engagement with reviewers will be
> +the same; the review process will be covered after the sections on GitGitGadget
> +and `git send-email`.
> +
> +== 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.

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

> +
> +To upload your branch to your own fork, you'll need to add the new fork as a
> +remote. You can use `git remote -v` to show the remotes you have added already.
> +From your new fork's page on GitHub, you can press "Clone or download" to get
> +the URL; then you need to run the following to add, replacing your own URL and
> +remote name for the examples provided:
> +
> +----
> +$ git remote add remotename git@github.com:remotename/git.git
> +----
> +
> +or to use the HTTPS URL:
> +
> +----
> +$ git remote add remotename https://github.com/remotename/git/.git
> +----
> +
> +Run `git remote -v` again and you should see the new remote showing up.
> +`git fetch remotename` (with the real name of your remote replaced) in order to
> +get ready to push.
> +
> +Next, double-check that you've been doing all your development in a new branch
> +by running `git branch`. If you didn't, now is a good time to move your new
> +commits to their own branch.
> +
> +As mentioned briefly at the beginning of this document, we are basing our work
> +on `master`, so go ahead and update as shown below, or using your preferred
> +workflow.
> +
> +----
> +$ git checkout master
> +$ git pull -r
> +$ git rebase master psuh
> +----
> +
> +Finally, you're ready to push your new topic branch! (Due to our branch and
> +command name choices, be careful when you type the command below.)
> +
> +----
> +$ git push remotename psuh
> +----
> +
> +Now you should be able to go and check out your newly created branch on GitHub.
> +
> +=== Sending a PR to GitGitGadget
> +
> +In order to have your code tested and formatted for review, you need to start by
> +opening a Pull Request against `gitgitgadget/git`. Head to
> +https://github.com/gitgitgadget/git and open a PR either with the "New pull
> +request" button or the convenient "Compare & pull request" button that may
> +appear with the name of your newly pushed branch.
> +
> +Review the PR's title and description, as it's used by GitGitGadget as the cover
> +letter for your change. When you're happy, submit your pull request.
> +
> +=== Running CI and Getting Ready to Send
> +
> +If it's your first time using GitGitGadget (which is likely, as you're using
> +this tutorial) then someone will need to give you permission to use the tool.
> +As mentioned in the GitGitGadget documentation, you just need someone who
> +already uses it to comment on your PR with `/allow <username>`. GitGitGadget
> +will automatically run your PRs through the CI even without the permission given
> +but you will not be able to `/submit` your changes until someone allows you to
> +use the tool.
> +
> +If the CI fails, you can update your changes with `git rebase -i` and push your
> +branch again:
> +
> +----
> +$ git push -f remotename psuh
> +----
> +
> +In fact, you should continue to make changes this way up until the point when
> +your patch is accepted into `next`.
> +
> +////
> +TODO https://github.com/gitgitgadget/gitgitgadget/issues/83
> +It'd be nice to be able to verify that the patch looks good before sending it
> +to everyone on Git mailing list.
> +=== Check Your Work
> +////
> +
> +=== 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"

> +commenting on your PR with `/submit`.
> +
> +=== Updating With Comments
> +
> +Skip ahead to <<reviewing,Responding to Reviews>> for information on how to
> +reply to review comments you will receive on the mailing list.
> +
> +Once you have your branch again in the shape you want following all review
> +comments, you can submit again:
> +
> +----
> +$ git push -f remotename psuh
> +----
> +
> +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"

> +to modify your description at the top of the pull request thread; it will be
> +used again as the cover letter. You should use this space to describe what
> +has changed since your previous version, so that your reviewers have some idea
> +of what they're looking at. When the CI is done running, you can comment once
> +more with `/submit` - GitGitGadget will automatically add a v2 mark to your
> +changes.
> +
> +== Sending Patches with `git send-email`
> +
> +If you don't want to use GitGitGadget, you can also use Git itself to mail your
> +patches. Some benefits of using Git this way include finer grained control of
> +subject line (for example, being able to use the tag [RFC PATCH] in the subject)
> +and being able to send a ``dry run'' mail to yourself to ensure it all looks
> +good before going out to the list.
> +
> +=== Prerequisite: Setting Up `git send-email`
> +
> +Configuration for `send-email` can vary based on your operating system and email
> +provider, and so will not be covered in this tutorial, beyond stating that in
> +many distributions of Linux, `git-send-email` is not packaged alongside the
> +typical `git` install. You may need to install this additional package; there
> +are a number of resources online to help you do so. You will also need to
> +determine the right way to configure it to use your SMTP server; again, as this
> +configuration can change significantly based on your system and email setup, it
> +is out of scope for the context of this tutorial.
> +
> +=== Preparing initial patchset
> +
> +Sending emails with Git is a two-part process; before you can prepare the emails
> +themselves, you'll need to prepare the patches. Luckily, this is pretty simple:
> +
> +----
> +$ git format-patch --cover-letter -o psuh/ master..psuh
> +----
> +
> +The `--cover-letter` parameter tells `format-patch` to create a cover letter
> +template for you. You will need to fill in the template before you're ready
> +to send - but for now, the template will be next to your other patches.
> +
> +The `-o psuh/` parameter tells `format-patch` to place the patch files into a
> +directory. This is useful because `git send-email` can take a directory and
> +send out all the patches from there.
> +
> +`master..psuh` tells `format-patch` to generate patches for the difference
> +between `master` and `psuh`. It will make one patch file per commit. After you
> +run, you can go have a look at each of the patches with your favorite text
> +editor and make sure everything looks alright; however, it's not recommended to
> +make code fixups via the patch file. It's a better idea to make the change the
> +normal way using `git rebase -i` or by adding a new commit than by modifying a
> +patch.
> +
> +NOTE: Optionally, you can also use the `--rfc` flag to prefix your patch subject
> +with ``[RFC PATCH]'' instead of ``[PATCH]''. RFC stands for ``request for
> +comments'' and indicates that while your code isn't quite ready for submission,
> +you'd like to begin the code review process. This can also be used when your
> +patch is a proposal, but you aren't sure whether the community wants to solve
> +the problem with that approach or not - to conduct a sort of design review. You
> +may also see on the list patches marked ``WIP'' - this means they are incomplete
> +but want reviewers to look at what they have so far. You can add this flag with
> +`--subject-prefix=WIP`.
> +
> +Check and make sure that your patches and cover letter template exist in the
> +directory you specified - you're nearly ready to send out your review!
> +
> +=== Preparing email
> +
> +In addition to an email per patch, the Git community also expects your patches
> +to come with a cover letter, typically with a subject line [PATCH 0/x] (where
> +x is the number of patches you're sending). Since you invoked `format-patch`
> +with `--cover-letter`, you've already got a template ready. Open it up in your
> +favorite editor.
> +
> +You should see a number of headers present already. Check that your `From:`
> +header is correct. Then modify your `Subject:` to something which succinctly
> +covers the purpose of your entire topic branch, for example:
> +
> +----
> +Subject: [PATCH 0/7] adding the 'psuh' command
> +----
> +
> +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.

> +parameters when you invoke `git send-email` to add the cover letter.
> +
> +Next you'll have to fill out the body of your cover letter. This is an important
> +component of change submission as it explains to the community from a high level
> +what you're trying to do, and why, in a way that's more apparent than just
> +looking at your diff. Be sure to explain anything your diff doesn't make clear
> +on its own.
> +
> +Here's an example body for `psuh`:
> +
> +----
> +Our internal metrics indicate widespread interest in the command
> +git-psuh - that is, many users are trying to use it, but finding it is
> +unavailable, using some unknown workaround instead.
> +
> +The following handful of patches add the psuh command and implement some
> +handy features on top of it.
> +
> +This patchset is part of the MyFirstContribution tutorial and should not
> +be merged.
> +----
> +
> +The template created by `git format-patch --cover-letter` includes a diffstat.
> +This gives reviewers a summary of what they're in for when reviewing your topic.
> +The one generated for `psuh` from the sample implementation looks like this:
> +
> +----
> + Documentation/git-psuh.txt | 40 +++++++++++++++++++++
> + Makefile                   |  1 +
> + builtin.h                  |  1 +
> + builtin/psuh.c             | 73 ++++++++++++++++++++++++++++++++++++++
> + git.c                      |  1 +
> + t/t9999-psuh-tutorial.sh   | 12 +++++++
> + 6 files changed, 128 insertions(+)
> + create mode 100644 Documentation/git-psuh.txt
> + create mode 100644 builtin/psuh.c
> + create mode 100755 t/t9999-psuh-tutorial.sh
> +----
> +
> +Finally, the letter will include the version of Git used to generate the
> +patches. You can leave that string alone.
> +
> +=== Sending email
> +
> +At this point you should have a directory `psuh/` which is filled with your
> +patches and a cover letter. Time to mail it out! You can send it like this:
> +
> +----
> +$ git send-email --to=target@example.com
> +----
> +
> +NOTE: Check `git help send-email` for some other options which you may find
> +valuable, such as changing the Reply-to address or adding more CC and BCC lines.
> +
> +NOTE: When you are sending a real patch, it will go to git@vger.kernel.org - but
> +please don't send your patchset from the tutorial to the real mailing list! For
> +now, you can send it to yourself, to make sure you understand how it will look.
> +
> +After you run the command above, you will be presented with an interactive
> +prompt for each patch that's about to go out. This gives you one last chance to
> +edit or quit sending something (but again, don't edit code this way). Once you
> +press `y` or `a` at these prompts your emails will be sent! Congratulations!
> +
> +Awesome, now the community will drop everything and review your changes. (Just
> +kidding - be patient!)
> +
> +=== Sending v2
> +
> +Skip ahead to <<reviewing,Responding to Reviews>> for information on how to
> +handle comments from reviewers. Continue this section when your topic branch is
> +shaped the way you want it to look for your patchset v2.
> +
> +When you're ready with the next iteration of your patch, the process is fairly
> +similar.
> +
> +First, generate your v2 patches again:
> +
> +----
> +$ git format-patch -v2 --cover-letter -o psuh/ master..psuh
> +----
> +
> +This will add your v2 patches, all named like `v2-000n-my-commit-subject.patch`,
> +to the `psuh/` directory. You may notice that they are sitting alongside the v1
> +patches; that's fine, but be careful when you are ready to send them.
> +
> +Edit your cover letter again. Now is a good time to mention what's different
> +between your last version and now, if it's something significant. You do not
> +need the exact same body in your second cover letter; focus on explaining to
> +reviewers the changes you've made that may not be as visible.
> +
> +You will also need to go and find the Message-Id of your previous cover letter.
> +You can either note it when you send the first series, from the output of `git
> +send-email`, or you can look it up on the
> +https://public-inbox.org/git[mailing list]. Find your cover letter in the
> +archives, click on it, then click "permalink" or "raw" to reveal the Message-Id
> +header. It should match:
> +
> +----
> +Message-Id: <foo.12345.author@example.com>
> +----
> +
> +Your Message-Id is `<foo.12345.author@example.com>`. This example will be used
> +below as well; make sure to replace it with the correct Message-Id for your
> +**previous cover letter** - that is, if you're sending v2, use the Message-Id
> +from v1; if you're sending v3, use the Message-Id from v2.
> +
> +While you're looking at the email, you should also note who is CC'd, as it's
> +common practice in the mailing list to keep all CCs on a thread. You can add
> +these CC lines directly to your cover letter with a line like so in the header
> +(before the Subject line):
> +
> +----
> +CC: author@example.com, Othe R <other@example.com>
> +----
> +
> +Now send the emails again, paying close attention to which messages you pass in
> +to the command:
> +
> +----
> +$ git send-email --to=target@example.com
> +                --in-reply-to=<foo.12345.author@example.com>

You probably need quotes around this message-id argument to avoid the
shell interpreting it as redirection.

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

> +why) of your patch, but if you need to supply even more context, you can do so
> +below the `---` in your patch. Take the example below, generated with
> +`git format-patch` on a single commit:
> +
> +----
> +From 1345bbb3f7ac74abde040c12e737204689a72723 Mon Sep 17 00:00:00 2001
> +From: A U Thor <author@example.com>
> +Date: Thu, 18 Apr 2019 15:11:02 -0700
> +Subject: [PATCH] README: change the grammar
> +
> +I think it looks better this way. This part of the commit message will
> +end up in the commit-log.
> +
> +Signed-off-by: A U Thor <author@example.com>
> +---
> +Let's have a wild discussion about grammar on the mailing list. This
> +part of my email will never end up in the commit log. Here is where I
> +can add additional context to the mailing list about my intent, outside
> +of the context of the commit log.
> +
> + README.md | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/README.md b/README.md
> +index 88f126184c..38da593a60 100644
> +--- a/README.md
> ++++ b/README.md
> +@@ -3,7 +3,7 @@
> + Git - fast, scalable, distributed revision control system
> + =========================================================
> +
> +-Git is a fast, scalable, distributed revision control system with an
> ++Git is a fast, scalable, and distributed revision control system with an
> + unusually rich command set that provides both high-level operations
> + and full access to internals.
> +
> +--
> +2.21.0.392.gf8f6787159e-goog
> +----
> +
> +== My Patch Got Emailed - Now What?
> +
> +[[reviewing]]
> +=== Responding to Reviews
> +
> +After a few days, you will hopefully receive a reply to your patchset with some
> +comments. Woohoo! Now you can get back to work.
> +
> +It's good manners to reply to each comment, notifying the reviewer that you have
> +made the change requested, feel the original is better, or that the comment
> +inspired you to do something a new way which is superior to both the original
> +and the suggested change. This way reviewers don't need to inspect your v2 to
> +figure out whether you implemented their comment or not.
> +
> +If you are going to push back on a comment, be polite and explain why you feel
> +your original is better; be prepared that the reviewer may still disagree with
> +you, and the rest of the community may weigh in on one side or the other. As
> +with all code reviews, it's important to keep an open mind to doing something a
> +different way than you originally planned; other reviewers have a different
> +perspective on the project than you do, and may be thinking of a valid side
> +effect which had not occurred to you. It is always okay to ask for clarification
> +if you aren't sure why a change was suggested, or what the reviewer is asking
> +you to do.
> +
> +Make sure your email client has a plaintext email mode and it is turned on; the
> +Git list rejects HTML email. Please also follow the mailing list etiquette
> +outlined in the
> +https://kernel.googlesource.com/pub/scm/git/git/+/todo/MaintNotes[Maintainer's
> +Note], which are similar to etiquette rules in most open source communities
> +surrounding bottom-posting and inline replies.
> +
> +When you're making changes to your code, it is cleanest - that is, the resulting
> +commits are easiest to look at - if you use `git rebase -i` (interactive
> +rebase). Take a look at this
> +https://www.oreilly.com/library/view/git-pocket-guide/9781449327507/ch10.html[overview]
> +from O'Reilly. The general idea is to modify each commit which requires changes;
> +this way, instead of having a patch A with a mistake, a patch B which was fine
> +and required no upstream reviews in v1, and a patch C which fixes patch A for
> +v2, you can just ship a v2 with a correct patch A and correct patch B. This is
> +changing history, but since it's local history which you haven't shared with
> +anyone, that is okay for now! (Later, it may not make sense to do this; take a
> +look at the section below this one for some context.)
> +
> +=== After Review Approval
> +
> +The Git project has four integration branches: `pu`, `next`, `master`, and
> +`maint`. Your change will be placed into `pu` fairly early on by the maintainer
> +while it is still in the review process; from there, when it is ready for wider
> +testing, it will be merged into `next`. Plenty of early testers use `next` and
> +may report issues. Eventually, changes in `next` will make it to `master`,
> +which is typically considered stable. Finally, when a new release is cut,
> +`maint` is used to base bugfixes onto. As mentioned at the beginning of this
> +document, you can read `Documents/SubmittingPatches` for some more info about
> +the use of the various integration branches.
> +
> +Back to now: your code has been lauded by the upstream reviewers. It is perfect.
> +It is ready to be accepted. You don't need to do anything else; the maintainer
> +will merge your topic branch to `next` and life is good.
> +
> +However, if you discover it isn't so perfect after this point, you may need to
> +take some special steps depending on where you are in the process.
> +
> +If the maintainer has announced in the "What's cooking in git.git" email that
> +your topic is marked for `next` - that is, that they plan to merge it to `next`
> +but have not yet done so - you should send an email asking the maintainer to
> +wait a little longer: "I've sent v4 of my series and you marked it for `next`,
> +but I need to change this and that - please wait for v5 before you merge it."
> +
> +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"

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

> +diffs from `<topic>..<mybranch>` and base your work on `<topic>` instead of
> +`master`.
> --
> 2.21.0.593.g511ec345e18-goog
>

  parent reply	other threads:[~2019-05-03  2:11 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 18:32 [PATCH 0/1] documentation: add lab for first contribution Emily Shaffer via GitGitGadget
2019-04-11 18:32 ` [PATCH 1/1] " Emily Shaffer via GitGitGadget
2019-04-12  3:20   ` Junio C Hamano
2019-04-12 22:03     ` Emily Shaffer
2019-04-13  5:39       ` Junio C Hamano
2019-04-15 17:26         ` Emily Shaffer
2019-04-11 21:03 ` [PATCH 0/1] " Josh Steadmon
2019-04-12  2:35 ` Junio C Hamano
2019-04-12 22:58   ` Emily Shaffer
2019-04-16 20:26 ` [PATCH v2 " Emily Shaffer via GitGitGadget
2019-04-16 20:26   ` [PATCH v2 1/1] " Emily Shaffer via GitGitGadget
2019-04-17  5:32     ` Junio C Hamano
2019-04-17  8:07       ` Eric Sunshine
2019-04-18  0:05         ` Junio C Hamano
2019-04-17 23:16       ` Emily Shaffer
2019-04-16 21:13   ` [PATCH v2 0/1] " Emily Shaffer
2019-04-19 16:57   ` [PATCH v3] " Emily Shaffer
2019-04-21 10:52     ` Junio C Hamano
2019-04-22 22:27       ` Emily Shaffer
2019-04-23 19:34     ` [PATCH v4] documentation: add tutorial " Emily Shaffer
2019-04-30 18:59       ` Josh Steadmon
2019-05-02  0:57         ` Emily Shaffer
2019-05-03  2:11       ` Phil Hord [this message]
2019-05-07 19:05         ` Emily Shaffer
2019-05-06 22:28       ` Jonathan Tan
2019-05-07 19:59         ` Emily Shaffer
2019-05-07 20:32           ` Jonathan Tan
2019-05-08  2:45         ` Junio C Hamano
2019-05-07 21:30       ` [PATCH v5 0/2] documentation: add lab " Emily Shaffer
2019-05-07 21:30         ` [PATCH v5 1/2] documentation: add tutorial " Emily Shaffer
2019-05-07 23:25           ` Emily Shaffer
2019-05-08  3:46           ` Junio C Hamano
2019-05-08 18:58             ` Emily Shaffer
2019-05-08 19:53               ` Jonathan Tan
2019-05-07 21:30         ` [PATCH v5 2/2] documentation: add anchors to MyFirstContribution Emily Shaffer
2019-05-08  3:30         ` [PATCH v5 0/2] documentation: add lab for first contribution Junio C Hamano
2019-05-17 19:03         ` [PATCH v6 0/2] documentation: add tutorial " Emily Shaffer
2019-05-17 19:07           ` [PATCH v6 1/2] " Emily Shaffer
2019-05-26  7:48             ` Christian Couder
2019-05-29 20:09               ` Emily Shaffer
2019-10-18 16:40             ` SZEDER Gábor
2019-10-18 22:54               ` Emily Shaffer
2019-05-17 19:07           ` [PATCH v6 2/2] documentation: add anchors to MyFirstContribution Emily Shaffer
2019-05-29 20:18           ` [PATCH] doc: add some nit fixes " Emily Shaffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABURp0rE23SCxB4VD0-kVWp6OfS7-4O6biyD7zMqSUQvR_RZxg@mail.gmail.com \
    --to=phil.hord@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).