threads / patch / 52124

patch, 3 partssome clarifications to MyFirstContribution

Subject: [PATCH 0/3] some clarifications to MyFirstContribution

## tl;dr

20 messages between Oct 26, 2019 and Oct 31, 2019. Diffs are folded; open one to read it.

replies: 19people: 6as markdown or json

Emily Shaffer· Oct 26, 2019, 00:51 UTC · lore

Hi all,

With the wave of Outreachy participants (hello! welcome!) a few shortcomings in the MyFirstContribution tutorial have been noted; plus, a comment from SZEDER (thanks!) sent along some weeks ago about a missing step in the new command process.

[1/3] Per SZEDER's comment, we should be adding `git psuh` to the list
      of commands used by 'git help'.
[2/3] Outreachy applicant Heba Waly indicated that the tutorial doesn't
      cover dependencies needed to build Git. However, I don't have a
      good handle on what's needed for platforms besides Linux, so I
      really appreciate others suggesting dependency installation steps
      for other platforms (even if it is a link to another guide).
[3/3] Per some discussions in #git-devel this week, it's not apparent
      who to ask for the /allow comment to tell GitGitGadget that you
      can be trusted. So, add the same hint which is proposed to be
      shown in GitGitGadget's welcome message.

Emily Shaffer (3):
  myfirstcontrib: add 'psuh' to command-list.txt
  myfirstcontrib: add dependency installation step
  myfirstcontrib: hint to find gitgitgadget allower

 Documentation/MyFirstContribution.txt | 57 +++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 3 deletions(-)

-- 
2.24.0.rc0.303.g954a862665-goog

Emily Shaffer· Oct 26, 2019, 00:51 UTC · re: Emily Shaffer · lore

[PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

Users can discover commands and their brief usage by running 'git help git' or 'git help -a'; both of these pages list all available commands based on the contents of 'command-list.txt'. That means adding a new command there is an important part of the new command process, and therefore belongs in the new command tutorial.

Teach new users how to add their command, and include a brief overview of how to discover which attributes to place on the command in the list.

Since 'git psuh' prints some workspace info, doesn't modify anything, and is targeted as a user-facing porcelain command, list it as a 'mainporcelain' and 'info' command.

As the usage string is required to generate this documentation, don't add the command to the list until after the usage string is added to the tutorial.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Show changes to Documentation/MyFirstContribution.txt +22 −0

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 5e9b808f5f..12b7256454 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -534,6 +534,28 @@ 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.
 
+Now that you have a usage hint, you can teach Git how to show it in the general
+command list shown by `git help git` or `git help -a`, which is generated from
+`command-list.txt`. Find the line for 'git-pull' so you can add your 'git-psuh'
+line above it in alphabetical order. Now, we can add some attributes about the
+command which impacts where it shows up in the aforementioned help commands. The
+top of `command-list.txt` shares some information about what each attribute
+means; in those help pages, the commands are sorted according to these
+attributes. `git psuh` is user-facing, or porcelain - so we will mark it as
+"mainporcelain". For "mainporcelain" commands, the comments at the top of
+`command-list.txt` indicate we can also optionally add an attribute from another
+list; since `git psuh` shows some information about the user's workspace but
+doesn't modify anything, let's mark it as "info". Make sure to keep your
+attributes in the same style as the rest of `command-list.txt` using spaces to
+align and delineate them:
+
+----
+git-prune-packed                        plumbingmanipulators
+git-psuh                                mainporcelain		info
+git-pull                                mainporcelain           remote
+git-push                                mainporcelain           remote
+----
+
 Build again. Now, when you run with `-h`, you should see your usage printed and
 your command terminated before anything else interesting happens. Great!

-- 
2.24.0.rc0.303.g954a862665-goog

Jonathan Nieder· Oct 26, 2019, 01:08 UTC · re: Emily Shaffer · lore

Re: [PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

Hi,

Emily Shaffer wrote:

Show 5 quoted lines

> Users can discover commands and their brief usage by running 'git help
> git' or 'git help -a'; both of these pages list all available commands
> based on the contents of 'command-list.txt'. That means adding a new
> command there is an important part of the new command process, and
> therefore belongs in the new command tutorial.

Makes sense.

Not about this patch: is there a way to detect this automatically? E.g. if a command in git.c::commands doesn't appear in command-list.txt, could we make Git fail "make test"?

[...]

Show 11 quoted lines

> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -534,6 +534,28 @@ 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.
>  
> +Now that you have a usage hint, you can teach Git how to show it in the general
> +command list shown by `git help git` or `git help -a`, which is generated from
> +`command-list.txt`. Find the line for 'git-pull' so you can add your 'git-psuh'
> +line above it in alphabetical order. Now, we can add some attributes about the
> +command which impacts where it shows up in the aforementioned help commands. The

nit: s/impacts/impact/, to agree with "attributes"

> +top of `command-list.txt` shares some information about what each attribute
> +means; in those help pages, the commands are sorted according to these
> +attributes. `git psuh` is user-facing, or porcelain - so we will mark it as

                                             ^^^^^^^^^

optional: This might be an unfamiliar term to people not thinking of the plumbing fixture / chrome analogy. Is there anything we should do to help them understand what's going on?

E.g. "git help glossary" says

	Porcelains expose more of a SCM interface than the plumbing.

Show 10 quoted lines

> +"mainporcelain". For "mainporcelain" commands, the comments at the top of
> +`command-list.txt` indicate we can also optionally add an attribute from another
> +list; since `git psuh` shows some information about the user's workspace but
> +doesn't modify anything, let's mark it as "info". Make sure to keep your
> +attributes in the same style as the rest of `command-list.txt` using spaces to
> +align and delineate them:
> +
> +----
> +git-prune-packed                        plumbingmanipulators
> +git-psuh                                mainporcelain		info

tabs snuck in.

> +git-pull                                mainporcelain           remote
> +git-push                                mainporcelain           remote
> +----
> +

The rest looks good.

Thanks, Jonathan

SZEDER Gábor· Oct 26, 2019, 08:00 UTC · re: Jonathan Nieder · lore

Re: [PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

On Fri, Oct 25, 2019 at 06:08:57PM -0700, Jonathan Nieder wrote:

Show 11 quoted lines

> > Users can discover commands and their brief usage by running 'git help
> > git' or 'git help -a'; both of these pages list all available commands
> > based on the contents of 'command-list.txt'. That means adding a new
> > command there is an important part of the new command process, and
> > therefore belongs in the new command tutorial.
> 
> Makes sense.
> 
> Not about this patch: is there a way to detect this automatically?
> E.g. if a command in git.c::commands doesn't appear in
> command-list.txt, could we make Git fail "make test"?

We almost detect this already:

  $ sed -i -e '/^git-bisect/d' command-list.txt 
  $ make check-docs
  make -C Documentation lint-docs
  make[1]: Entering directory '/home/szeder/src/git/Documentation'
      GEN cmd-list.made
      GEN doc.dep
  make[2]: Entering directory '/home/szeder/src/git'
  make[2]: 'GIT-VERSION-FILE' is up to date.
  make[2]: Leaving directory '/home/szeder/src/git'
  make[2]: Entering directory '/home/szeder/src/git'
  make[2]: 'GIT-VERSION-FILE' is up to date.
  make[2]: Leaving directory '/home/szeder/src/git'
      LINT lint-docs
  make[1]: Leaving directory '/home/szeder/src/git/Documentation'
  no link: git-bisect
  $ echo $?
  0

See that last "no link: git-bisect" line? That's what happened to catch my eyes when Derrick forgot to add his new 'sparse-checkout' builtin to 'command-list.txt'. I still haven't looked up what that 'no link' is supposed to mean, but if it were an error, then we would have surely detected the missing entry in 'command-list.txt' in out CI builds.

Another possibility would be to auto-generate that long list of 'cmd_foo()' function declaration in 'builtin.h' from 'command-list.txt', by adding a 'builtin' attribute to mark builtin commands.

Junio C Hamano· Oct 28, 2019, 01:24 UTC · re: SZEDER Gábor · lore

Re: [PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

SZEDER Gábor <szeder.dev@gmail.com> writes:

> See that last "no link: git-bisect" line?  That's what happened to
> catch my eyes when Derrick forgot to add his new 'sparse-checkout'
> builtin to 'command-list.txt'.  I still haven't looked up what that
> 'no link' is supposed to mean, but if it were an error, then we would

Build procedure for the Documentation uses ../command-list.txt as its input to produce cmds-<class>.txt files, that are included in git.txt (hence resulting git.1 and git.html). While it works, it tries to make sure that a command that has its own documentation page at Documentation/git-<command>.txt is listed; otherwise we have a page for <command> to which there is no link from git.{1,html}.

I think "make check-builtins" might also want to sanity check command-list.txt.

SZEDER Gábor· Oct 28, 2019, 11:25 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

On Mon, Oct 28, 2019 at 10:24:31AM +0900, Junio C Hamano wrote:

Show 13 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > See that last "no link: git-bisect" line?  That's what happened to
> > catch my eyes when Derrick forgot to add his new 'sparse-checkout'
> > builtin to 'command-list.txt'.  I still haven't looked up what that
> > 'no link' is supposed to mean, but if it were an error, then we would
> 
> Build procedure for the Documentation uses ../command-list.txt as
> its input to produce cmds-<class>.txt files, that are included in
> git.txt (hence resulting git.1 and git.html).  While it works, it
> tries to make sure that a command that has its own documentation
> page at Documentation/git-<command>.txt is listed; otherwise we have
> a page for <command> to which there is no link from git.{1,html}.

Oh, then it doesn't quite do what I thought it does.

> I think "make check-builtins" might also want to sanity check
> command-list.txt.

I haven't noticed that we have something like that. FWIW, our documentation CI job runs it already.

And indeed it can be easily extended to check 'command-list.txt' as well, but then it will find some other builtins not included in 'command-list.txt', namely:

  bisect--helper env--helper fsck-objects init-db merge-ours
  merge-recursive merge-subtree remote-ext remote-fd submodule--helper

I think it makes sense not to include the '*--helper' commands, or 'fsck-objects' and 'init-db', but I'm not sure about the others.

OTOH, it won't help if we add a new git-foo script and forgot to include it in 'command-list.txt'.

Johannes Schindelin· Oct 29, 2019, 20:39 UTC · re: SZEDER Gábor · lore

Re: [PATCH 1/3] myfirstcontrib: add 'psuh' to command-list.txt

Hi Gábor,

On Sat, 26 Oct 2019, SZEDER Gábor wrote:

Show 39 quoted lines

> On Fri, Oct 25, 2019 at 06:08:57PM -0700, Jonathan Nieder wrote:
> > > Users can discover commands and their brief usage by running 'git help
> > > git' or 'git help -a'; both of these pages list all available commands
> > > based on the contents of 'command-list.txt'. That means adding a new
> > > command there is an important part of the new command process, and
> > > therefore belongs in the new command tutorial.
> >
> > Makes sense.
> >
> > Not about this patch: is there a way to detect this automatically?
> > E.g. if a command in git.c::commands doesn't appear in
> > command-list.txt, could we make Git fail "make test"?
>
> We almost detect this already:
>
>   $ sed -i -e '/^git-bisect/d' command-list.txt
>   $ make check-docs
>   make -C Documentation lint-docs
>   make[1]: Entering directory '/home/szeder/src/git/Documentation'
>       GEN cmd-list.made
>       GEN doc.dep
>   make[2]: Entering directory '/home/szeder/src/git'
>   make[2]: 'GIT-VERSION-FILE' is up to date.
>   make[2]: Leaving directory '/home/szeder/src/git'
>   make[2]: Entering directory '/home/szeder/src/git'
>   make[2]: 'GIT-VERSION-FILE' is up to date.
>   make[2]: Leaving directory '/home/szeder/src/git'
>       LINT lint-docs
>   make[1]: Leaving directory '/home/szeder/src/git/Documentation'
>   no link: git-bisect
>   $ echo $?
>   0
>
> See that last "no link: git-bisect" line?  That's what happened to
> catch my eyes when Derrick forgot to add his new 'sparse-checkout'
> builtin to 'command-list.txt'.  I still haven't looked up what that
> 'no link' is supposed to mean, but if it were an error, then we would
> have surely detected the missing entry in 'command-list.txt' in out CI
> builds.

FWIW I think the only reason that this is not an error in `check-docs` (which _is_ run by our CI/PR builds, in the `Documentation` job) is that it used to be buggy.

However, I think I managed to address the remaining issues in `js/misc-doc-fixes` (and maybe there were a couple spill-overs into `js/check-docs-exe`).

In short: I think we can make this type of issue trigger an error in `check-docs` now (as in: non-zero exit code).

Ciao, Dscho

Show 7 quoted lines

>
> Another possibility would be to auto-generate that long list of
> 'cmd_foo()' function declaration in 'builtin.h' from
> 'command-list.txt', by adding a 'builtin' attribute to mark builtin
> commands.
>
>

Emily Shaffer· Oct 26, 2019, 00:51 UTC · re: Emily Shaffer · lore

[PATCH 2/3] myfirstcontrib: add dependency installation step

Indicate that the user needs some dependencies before the build will run happily on their machine; this dependency list doesn't seem to be made clear anywhere else in the project documentation. Then, so the user can be certain any build failures are due to their code and not their environment, perform a build on a clean checkout of 'master'. Also, move the note about build parallelization up here, so that it appears next to the very first build invocation in the tutorial.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Show changes to Documentation/MyFirstContribution.txt +24 −3

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 12b7256454..b8ffeda07e 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -38,6 +38,30 @@ $ git clone https://github.com/git/git git
 $ cd git
 ----
 
+[[dependencies]]
+=== Installing Dependencies
+
+To build Git from source, you need to have a handful of dependencies installed
+on your system. For a hint of what's needed, you can take a look at
+`ci/install-dependencies.sh`.
+
+To install the dependencies needed for a basic build on Linux, run something
+like this (or replace `apt` with your distribution's package manager of choice):
+
+----
+# apt install libssl-dev zlib1g-dev libcurl4-gnutls-dev libexpat1-dev
+----
+
+Make sure that your environment has everything you need by building your brand
+new clone of Git from the above step:
+
+----
+$ make
+----
+
+NOTE: The Git build is parallelizable. `-j#` is not included above but you can
+use it as you prefer, here and elsewhere.
+
 [[identify-problem]]
 === Identify Problem to Solve
 
@@ -138,9 +162,6 @@ 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.

-- 
2.24.0.rc0.303.g954a862665-goog

Jonathan Nieder· Oct 26, 2019, 01:12 UTC · re: Emily Shaffer · lore

Re: [PATCH 2/3] myfirstcontrib: add dependency installation step

Hi,

Emily Shaffer wrote:

> Indicate that the user needs some dependencies before the build will run
> happily on their machine; this dependency list doesn't seem to be made
> clear anywhere else in the project documentation.

In theory, this info should be in INSTALL. I wouldn't be surprised if it's missing some info, though.

[...]

Show 15 quoted lines

> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -38,6 +38,30 @@ $ git clone https://github.com/git/git git
>  $ cd git
>  ----
>  
> +[[dependencies]]
> +=== Installing Dependencies
> +
> +To build Git from source, you need to have a handful of dependencies installed
> +on your system. For a hint of what's needed, you can take a look at
> +`ci/install-dependencies.sh`.
> +
> +To install the dependencies needed for a basic build on Linux, run something
> +like this (or replace `apt` with your distribution's package manager of choice):

pedantic nit: s/or replace/replacing/ ("or" would mean rpm is an alternative to what came before, but "something like" in the phrase before has rpm already included)

> +
> +----
> +# apt install libssl-dev zlib1g-dev libcurl4-gnutls-dev libexpat1-dev

Perhaps build-essential, too, in case they're using a system not set up for development.

Show 24 quoted lines

> +----
> +
> +Make sure that your environment has everything you need by building your brand
> +new clone of Git from the above step:
> +
> +----
> +$ make
> +----
> +
> +NOTE: The Git build is parallelizable. `-j#` is not included above but you can
> +use it as you prefer, here and elsewhere.
> +
>  [[identify-problem]]
>  === Identify Problem to Solve
>  
> @@ -138,9 +162,6 @@ 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.

With whatever subset of the changes above make sense,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

Junio C Hamano· Oct 28, 2019, 01:27 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/3] myfirstcontrib: add dependency installation step

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 31 quoted lines

> Hi,
>
> Emily Shaffer wrote:
>
>> Indicate that the user needs some dependencies before the build will run
>> happily on their machine; this dependency list doesn't seem to be made
>> clear anywhere else in the project documentation.
>
> In theory, this info should be in INSTALL.  I wouldn't be surprised if
> it's missing some info, though.
>
> [...]
>> --- a/Documentation/MyFirstContribution.txt
>> +++ b/Documentation/MyFirstContribution.txt
>> @@ -38,6 +38,30 @@ $ git clone https://github.com/git/git git
>>  $ cd git
>>  ----
>>  
>> +[[dependencies]]
>> +=== Installing Dependencies
>> +
>> +To build Git from source, you need to have a handful of dependencies installed
>> +on your system. For a hint of what's needed, you can take a look at
>> +`ci/install-dependencies.sh`.
>> +
>> +To install the dependencies needed for a basic build on Linux, run something
>> +like this (or replace `apt` with your distribution's package manager of choice):
>
> pedantic nit: s/or replace/replacing/ ("or" would mean rpm is an
> alternative to what came before, but "something like" in the phrase
> before has rpm already included)

This section, as you hinted at the beginning, should be a single sentence, i.e. just "follow INSTALL". Any clarifications should be made as a patch to the INSTALL file.

Thanks.

Emily Shaffer· Oct 31, 2019, 20:58 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/3] myfirstcontrib: add dependency installation step

On Mon, Oct 28, 2019 at 10:27:25AM +0900, Junio C Hamano wrote:

Show 37 quoted lines

> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > Hi,
> >
> > Emily Shaffer wrote:
> >
> >> Indicate that the user needs some dependencies before the build will run
> >> happily on their machine; this dependency list doesn't seem to be made
> >> clear anywhere else in the project documentation.
> >
> > In theory, this info should be in INSTALL.  I wouldn't be surprised if
> > it's missing some info, though.
> >
> > [...]
> >> --- a/Documentation/MyFirstContribution.txt
> >> +++ b/Documentation/MyFirstContribution.txt
> >> @@ -38,6 +38,30 @@ $ git clone https://github.com/git/git git
> >>  $ cd git
> >>  ----
> >>  
> >> +[[dependencies]]
> >> +=== Installing Dependencies
> >> +
> >> +To build Git from source, you need to have a handful of dependencies installed
> >> +on your system. For a hint of what's needed, you can take a look at
> >> +`ci/install-dependencies.sh`.
> >> +
> >> +To install the dependencies needed for a basic build on Linux, run something
> >> +like this (or replace `apt` with your distribution's package manager of choice):
> >
> > pedantic nit: s/or replace/replacing/ ("or" would mean rpm is an
> > alternative to what came before, but "something like" in the phrase
> > before has rpm already included)
> 
> This section, as you hinted at the beginning, should be a single
> sentence, i.e. just "follow INSTALL".  Any clarifications should 
> be made as a patch to the INSTALL file.

Thanks. I ended up with a little over one sentence; I don't like that a newbie following INSTALL file would end up with their locally-built Git installed (and they may only be interested in a one-off contribution, and never fetch again, or something) so I tried to highlight the dependencies portion of INSTALL:

  To build Git from source, you need to have a handful of dependencies installed
  on your system. For a hint of what's needed, you can take a look at
  `INSTALL`, paying close attention to the section about Git's dependencies on
  external programs and libraries. That document mentions a way to "test-drive"
  our freshly built Git without installing; that's the method we'll be using in
  this tutorial.

  Make sure that your environment has everything you need by building your brand
  new clone of Git from the above step:

 - Emily

Emily Shaffer· Oct 26, 2019, 00:51 UTC · re: Emily Shaffer · lore

[PATCH 3/3] myfirstcontrib: hint to find gitgitgadget allower

GitGitGadget, a handy tool for converting pull requests against Git into Git-mailing-list-friendly-patch-emails, requires as anti-spam that all new users be "/allow"ed by an existing user once before it will do anything for that new user. While this tutorial explained that mechanism, it did not give much hint on how to go about finding someone to allow your new pull request. So, teach our new GitGitGadget user where to look for someone who can add their name to the list.

The advice in this patch is based on the advice proposed for
GitGitGadget: https://github.com/gitgitgadget/gitgitgadget/pull/138

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

Show changes to Documentation/MyFirstContribution.txt +8 −0

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index b8ffeda07e..2de06de026 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -789,6 +789,14 @@ 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.
 
+NOTE: You can typically find someone who can `/allow` you on GitGitGadget by
+either examining recent pull requests where someone has been granted `/allow`
+(https://github.com/gitgitgadget/git/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%22%2Fallow%22[Search:
+is:pr is:open "/allow"]), in which case both the author and the person who
+granted the `/allow` can now `/allow` you, or by inquiring on the
+https://webchat.freenode.net/#git-devel[#git-devel] IRC channel on Freenode
+linking your pull request and asking for someone to `/allow` you.
+
 If the CI fails, you can update your changes with `git rebase -i` and push your
 branch again:

-- 
2.24.0.rc0.303.g954a862665-goog

Jonathan Nieder· Oct 26, 2019, 01:15 UTC · re: Emily Shaffer · lore

Re: [PATCH 3/3] myfirstcontrib: hint to find gitgitgadget allower

Emily Shaffer wrote:

Show 15 quoted lines

> GitGitGadget, a handy tool for converting pull requests against Git into
> Git-mailing-list-friendly-patch-emails, requires as anti-spam that all
> new users be "/allow"ed by an existing user once before it will do
> anything for that new user. While this tutorial explained that
> mechanism, it did not give much hint on how to go about finding someone
> to allow your new pull request. So, teach our new GitGitGadget user
> where to look for someone who can add their name to the list.
>
> The advice in this patch is based on the advice proposed for
> GitGitGadget: https://github.com/gitgitgadget/gitgitgadget/pull/138
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  Documentation/MyFirstContribution.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Though I'd appreciate an ack from a gitgitgadget specialist too, so cc-ing Dscho.

Show 19 quoted lines

> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index b8ffeda07e..2de06de026 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -789,6 +789,14 @@ 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.
>  
> +NOTE: You can typically find someone who can `/allow` you on GitGitGadget by
> +either examining recent pull requests where someone has been granted `/allow`
> +(https://github.com/gitgitgadget/git/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%22%2Fallow%22[Search:
> +is:pr is:open "/allow"]), in which case both the author and the person who
> +granted the `/allow` can now `/allow` you, or by inquiring on the
> +https://webchat.freenode.net/#git-devel[#git-devel] IRC channel on Freenode
> +linking your pull request and asking for someone to `/allow` you.
> +
>  If the CI fails, you can update your changes with `git rebase -i` and push your
>  branch again:
>

Johannes Schindelin· Oct 29, 2019, 20:43 UTC · re: Jonathan Nieder · lore

Re: [PATCH 3/3] myfirstcontrib: hint to find gitgitgadget allower

Hi Jonathan,

On Fri, 25 Oct 2019, Jonathan Nieder wrote:

Show 22 quoted lines

> Emily Shaffer wrote:
>
> > GitGitGadget, a handy tool for converting pull requests against Git into
> > Git-mailing-list-friendly-patch-emails, requires as anti-spam that all
> > new users be "/allow"ed by an existing user once before it will do
> > anything for that new user. While this tutorial explained that
> > mechanism, it did not give much hint on how to go about finding someone
> > to allow your new pull request. So, teach our new GitGitGadget user
> > where to look for someone who can add their name to the list.
> >
> > The advice in this patch is based on the advice proposed for
> > GitGitGadget: https://github.com/gitgitgadget/gitgitgadget/pull/138
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> >  Documentation/MyFirstContribution.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Though I'd appreciate an ack from a gitgitgadget specialist too, so
> cc-ing Dscho.

Seeing as the suggestions from gitgitgadget/gitgitgadget#138 partially came from me (and that I merged that PR after reviewing it), this comes very easily:

Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thanks, Dscho

Show 21 quoted lines

>
> > diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> > index b8ffeda07e..2de06de026 100644
> > --- a/Documentation/MyFirstContribution.txt
> > +++ b/Documentation/MyFirstContribution.txt
> > @@ -789,6 +789,14 @@ 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.
> >
> > +NOTE: You can typically find someone who can `/allow` you on GitGitGadget by
> > +either examining recent pull requests where someone has been granted `/allow`
> > +(https://github.com/gitgitgadget/git/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%22%2Fallow%22[Search:
> > +is:pr is:open "/allow"]), in which case both the author and the person who
> > +granted the `/allow` can now `/allow` you, or by inquiring on the
> > +https://webchat.freenode.net/#git-devel[#git-devel] IRC channel on Freenode
> > +linking your pull request and asking for someone to `/allow` you.
> > +
> >  If the CI fails, you can update your changes with `git rebase -i` and push your
> >  branch again:
> >
>

Pratyush Yadav· Oct 27, 2019, 01:39 UTC · re: Emily Shaffer · lore

Re: [PATCH 3/3] myfirstcontrib: hint to find gitgitgadget allower

On 25/10/19 05:51PM, Emily Shaffer wrote:

Show 16 quoted lines

> diff --git a/Documentation/MyFirstContribution.txt 
> b/Documentation/MyFirstContribution.txt
> index b8ffeda07e..2de06de026 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -789,6 +789,14 @@ 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.
>  
> +NOTE: You can typically find someone who can `/allow` you on GitGitGadget by
> +either examining recent pull requests where someone has been granted `/allow`
> +(https://github.com/gitgitgadget/git/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%22%2Fallow%22[Search:
> +is:pr is:open "/allow"]), in which case both the author and the person who
> +granted the `/allow` can now `/allow` you, or by inquiring on the
> +https://webchat.freenode.net/#git-devel[#git-devel] IRC channel on Freenode
> +linking your pull request and asking for someone to `/allow` you.

Nit: that's a lot to put in a single sentence. Maybe break it into parts 
like:

  NOTE: You can typically find someone who can `/allow` you on 
  GitGitGadget by looking at recent pull requests where someone has been 
  `/allow`ed. Both the author of that pull request and the person who 
  granted the permission can now `/allow` you. You can also inquire on 
  the IRC channel on Freenode linking your pull request and asking 
  someone to `/allow` you.

> +
>  If the CI fails, you can update your changes with `git rebase -i` and push your
>  branch again:

-- 
Regards,
Pratyush Yadav

Jonathan Nieder· Oct 26, 2019, 01:17 UTC · re: Emily Shaffer · lore

Re: [PATCH 0/3] some clarifications to MyFirstContribution

Emily Shaffer wrote:

> [1/3] Per SZEDER's comment, we should be adding `git psuh` to the list
>       of commands used by 'git help'.
> [2/3] Outreachy applicant Heba Waly indicated that the tutorial doesn't
>       cover dependencies needed to build Git.

Oh! It's also a good idea to give these people credit for their contribution, via a Reported-by in the commit message.

Sorry I missed it before (I tend to read the patches first and cover letter last, which is good for ensuring the commit messages are self contained enough to be helpful when the patch gets dug up in history later, but not so good for noticing this kind of thing).

Thanks, Jonathan

Emily Shaffer· Oct 31, 2019, 21:03 UTC · re: Emily Shaffer · lore

[PATCH v2 0/3] some clarifications to MyFirstContribution

With adjustments to install step based on email comments, plus Reported-by in command-list and install step patches.

 - Emily

Emily Shaffer (3):
  myfirstcontrib: add 'psuh' to command-list.txt
  myfirstcontrib: add dependency installation step
  myfirstcontrib: hint to find gitgitgadget allower

 Documentation/MyFirstContribution.txt | 53 +++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

-- 
2.24.0.rc0.303.g954a862665-goog

Emily Shaffer· Oct 31, 2019, 21:03 UTC · re: Emily Shaffer · lore

[PATCH v2 1/3] myfirstcontrib: add 'psuh' to command-list.txt

Teach new users how to add their command, and include a brief overview of how to discover which attributes to place on the command in the list.

Since 'git psuh' prints some workspace info, doesn't modify anything, and is targeted as a user-facing porcelain command, list it as a 'mainporcelain' and 'info' command.

As the usage string is required to generate this documentation, don't add the command to the list until after the usage string is added to the tutorial.

Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Show changes to Documentation/MyFirstContribution.txt +22 −0

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 5e9b808f5f..12b7256454 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -534,6 +534,28 @@ 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.
 
+Now that you have a usage hint, you can teach Git how to show it in the general
+command list shown by `git help git` or `git help -a`, which is generated from
+`command-list.txt`. Find the line for 'git-pull' so you can add your 'git-psuh'
+line above it in alphabetical order. Now, we can add some attributes about the
+command which impacts where it shows up in the aforementioned help commands. The
+top of `command-list.txt` shares some information about what each attribute
+means; in those help pages, the commands are sorted according to these
+attributes. `git psuh` is user-facing, or porcelain - so we will mark it as
+"mainporcelain". For "mainporcelain" commands, the comments at the top of
+`command-list.txt` indicate we can also optionally add an attribute from another
+list; since `git psuh` shows some information about the user's workspace but
+doesn't modify anything, let's mark it as "info". Make sure to keep your
+attributes in the same style as the rest of `command-list.txt` using spaces to
+align and delineate them:
+
+----
+git-prune-packed                        plumbingmanipulators
+git-psuh                                mainporcelain		info
+git-pull                                mainporcelain           remote
+git-push                                mainporcelain           remote
+----
+
 Build again. Now, when you run with `-h`, you should see your usage printed and
 your command terminated before anything else interesting happens. Great!

-- 
2.24.0.rc0.303.g954a862665-goog

Emily Shaffer· Oct 31, 2019, 21:03 UTC · re: Emily Shaffer · lore

[PATCH v2 2/3] myfirstcontrib: add dependency installation step

Reported-by: Heba Waly <heba.waly@gmail.com>
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Show changes to Documentation/MyFirstContribution.txt +20 −3

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 12b7256454..5ce94e077c 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -38,6 +38,26 @@ $ git clone https://github.com/git/git git
 $ cd git
 ----
 
+[[dependencies]]
+=== Installing Dependencies
+
+To build Git from source, you need to have a handful of dependencies installed
+on your system. For a hint of what's needed, you can take a look at
+`INSTALL`, paying close attention to the section about Git's dependencies on
+external programs and libraries. That document mentions a way to "test-drive"
+our freshly built Git without installing; that's the method we'll be using in
+this tutorial.
+
+Make sure that your environment has everything you need by building your brand
+new clone of Git from the above step:
+
+----
+$ make
+----
+
+NOTE: The Git build is parallelizable. `-j#` is not included above but you can
+use it as you prefer, here and elsewhere.
+
 [[identify-problem]]
 === Identify Problem to Solve
 
@@ -138,9 +158,6 @@ 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.

-- 
2.24.0.rc0.303.g954a862665-goog

Emily Shaffer· Oct 31, 2019, 21:03 UTC · re: Emily Shaffer · lore

[PATCH v2 3/3] myfirstcontrib: hint to find gitgitgadget allower

The advice in this patch is based on the advice proposed for
GitGitGadget: https://github.com/gitgitgadget/gitgitgadget/pull/138

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/MyFirstContribution.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

Show changes to Documentation/MyFirstContribution.txt +8 −0

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index 5ce94e077c..b55837e646 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -785,6 +785,14 @@ 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.
 
+NOTE: You can typically find someone who can `/allow` you on GitGitGadget by
+either examining recent pull requests where someone has been granted `/allow`
+(https://github.com/gitgitgadget/git/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+%22%2Fallow%22[Search:
+is:pr is:open "/allow"]), in which case both the author and the person who
+granted the `/allow` can now `/allow` you, or by inquiring on the
+https://webchat.freenode.net/#git-devel[#git-devel] IRC channel on Freenode
+linking your pull request and asking for someone to `/allow` you.
+
 If the CI fails, you can update your changes with `git rebase -i` and push your
 branch again:

-- 
2.24.0.rc0.303.g954a862665-goog