git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Victoria Dye via GitGitGadget <gitgitgadget@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Derrick Stolee <derrickstolee@github.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Josh Steadmon <steadmon@google.com>,
	Glen Choo <chooglen@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Victoria Dye <vdye@github.com>
Subject: Re: [PATCH v2] Documentation: add ReviewingGuidelines
Date: Mon, 19 Sep 2022 17:43:15 -0700	[thread overview]
Message-ID: <CABPp-BEB_+YoKZ=U6NPc8J+KZyMSYRsom34CeqjxUCyw0=LEyg@mail.gmail.com> (raw)
In-Reply-To: <pull.1348.v2.git.1663614767058.gitgitgadget@gmail.com>

On Mon, Sep 19, 2022 at 12:21 PM Victoria Dye via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
[...]
> +==== Performing your review
> +- Provide your review comments per-patch in a plaintext "Reply-All" email to the
> +  relevant patch. Comments should be made inline, immediately below the relevant
> +  section(s).
> +
> +- You may find that the limited context provided in the patch diff is sometimes
> +  insufficient for a thorough review. In such cases, you can review patches in
> +  your local tree by either applying patches with linkgit:git-am[1] or checking
> +  out the associated branch from https://github.com/gitster/git once the series
> +  is tracked there.

Lots of reviews also come with "Fetch-It-Via" instructions in the
cover letter, making it really easy to grab.  Might be worth
mentioning?

Also, would it make sense for us to replace "applying" with
"downloading and applying", perhaps mentioning `b4 am` for the
downloading half?

(I tend to use the Fetch-It-Via or wait for it to show up in
gitster/git, but b4 is really nice for the other cases.)

> +- Large, complicated patch diffs are sometimes unavoidable, such as when they
> +  refactor existing code. If you find such a patch difficult to parse, try
> +  reviewing the diff produced with the `--color-moved` and/or
> +  `--ignore-space-change` options.

Similarly, Documentation refactorings or significant rewordings are
sometimes easier to view with --color-words or --color-words=.

[...]
> +See Also
> +--------
> +link:MyFirstContribution.html[MyFirstContribution]
>
> base-commit: 79f2338b3746d23454308648b2491e5beba4beff

I like this document!  I had a couple ideas that might or might not
make sense to include in the document; it looks good to me either way.

  parent reply	other threads:[~2022-09-20  0:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-09 18:13 [PATCH] Documentation: add ReviewingGuidelines Victoria Dye via GitGitGadget
2022-09-09 19:42 ` Junio C Hamano
2022-09-13 17:54 ` Junio C Hamano
2022-09-13 23:11   ` Victoria Dye
2022-09-19 15:08     ` Derrick Stolee
2022-09-19 15:48       ` Johannes Schindelin
2022-09-15 23:25 ` Josh Steadmon
2022-09-19 18:17   ` Glen Choo
2022-09-19 19:12 ` [PATCH v2] " Victoria Dye via GitGitGadget
2022-09-19 20:15   ` Josh Steadmon
2022-09-19 21:37   ` Junio C Hamano
2022-09-20  0:43   ` Elijah Newren [this message]
2022-09-20 21:23     ` Konstantin Ryabitsev
2022-09-22  4:24     ` Shaoxuan Yuan
2022-09-22 13:29   ` Phillip Wood

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='CABPp-BEB_+YoKZ=U6NPc8J+KZyMSYRsom34CeqjxUCyw0=LEyg@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chooglen@google.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=steadmon@google.com \
    --cc=vdye@github.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).