[PATCH 0/4] update MyFirstContribution with current code base

[PATCH 0/4] update MyFirstContribution with current code base

[K Jayatheerth]

A series of patches extending my GSOC microproject
Fixing the documentation MyFirstContribution.adoc

Updated various commits since the last patches sent.

Feedbacks taken to write detailed commit messages

K Jayatheerth (4):
  Remove unused git-mentoring mailing list
  Docs: Correct cmd_psuh and Explain UNUSED macro
  Docs: Add cmd_psuh with repo and UNUSED removal
  cmd_psuh: Prefer repo_config for config lookup

 Documentation/MyFirstContribution.adoc | 89 ++++++++++++++++++--------
 1 file changed, 62 insertions(+), 27 deletions(-)

-- 
2.49.GIT

[PATCH 1/4] Remove unused git-mentoring mailing list

[K Jayatheerth]

This commit removes the details,
about the git-mentoring@googlegroups.com mailing list.

Reason: This mentoring mailing list is no longer actively used.
New contributors often use the main Git mailing list,
their queries almost always gets answered.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ca1d688c9b..ef190d8748 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
 The https://lore.kernel.org/git[archive] of this mailing list is
 available to view in a browser.
 
-==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
-
-This mailing list is targeted to new contributors and was created as a place to
-post questions and receive answers outside of the public eye of the main list.
-Veteran contributors who are especially interested in helping mentor newcomers
-are present on the list. In order to avoid search indexers, group membership is
-required to view messages; anyone can join and no approval is required.
-
 ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
 
 This IRC channel is for conversations between Git contributors. If someone is
-- 
2.49.GIT

[PATCH 2/4] Docs: Correct cmd_psuh and Explain UNUSED macro

[K Jayatheerth]

The `cmd_psuh` function signature in the documentation,
was missing the `struct repository *repo` argument,
which is standard for built-in commands.
This commit corrects the signature to include the `repo` argument.

Additionally, this commit adds an explanation,
for using the `UNUSED` macro to prevent compiler warnings.

This helps new contributors understand,
common practices in the Git codebase.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ef190d8748..b463d42f63 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -142,7 +142,15 @@ 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)
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
+----
+
+We also use the UNUSED macro to make sure we don't recieve compiler warnings
+for unused arguments from the function cmd_psuh.
+
+----
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
+			const char *prefix UNUSED, struct repository *repo UNUSED)
 ----
 
 We'll also need to add the declaration of psuh; open up `builtin.h`, find the
-- 
2.49.GIT

Re: [PATCH 2/4] Docs: Correct cmd_psuh and Explain UNUSED macro

[Emily Shaffer]

On Wed, Apr 16, 2025 at 11:44:48AM +0530, K Jayatheerth wrote:
> 
> The `cmd_psuh` function signature in the documentation,
> was missing the `struct repository *repo` argument,
> which is standard for built-in commands.
> This commit corrects the signature to include the `repo` argument.
> 
> Additionally, this commit adds an explanation,
> for using the `UNUSED` macro to prevent compiler warnings.
> 
> This helps new contributors understand,
> common practices in the Git codebase.

For this commit and others, take another look at the commit message
guidelines in
https://git-scm.com/docs/SubmittingPatches#describe-changes - this
commit message recaps "what" you are doing in the diff, which isn't
necessary because the diff is very simple to look at :) but it only
briefly explains "why". You did a good job explaining why in your cover
letter, IMO - but your cover letter doesn't live in the code base
forever. Your commit message does :)

> 
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ef190d8748..b463d42f63 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -142,7 +142,15 @@ 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)
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
> +----
> +
> +We also use the UNUSED macro to make sure we don't recieve compiler warnings
> +for unused arguments from the function cmd_psuh.

I think it's a good idea to mention that this is temporary - since you
are removing UNUSED in the very next step.

> +
> +----
> +int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
> +			const char *prefix UNUSED, struct repository *repo UNUSED)
>  ----
>  
>  We'll also need to add the declaration of psuh; open up `builtin.h`, find the
> -- 
> 2.49.GIT
> 
> 

[PATCH 3/4] Docs: Add cmd_psuh with repo and UNUSED removal

[K Jayatheerth]

This commit improves the `cmd_psuh` documentation example by:

Correcting the function signature to include struct repository *repo.
Makes the signature accurate and consistent with typical Git built-in
commands.

Removing the `UNUSED` macros from the `cmd_psuh` function arguments
(argc, argv, prefix, repo). This is done because the example now
uses these arguments.

Showing how to access the repository's Git directory (repo->gitdir)
within the cmd_psuh function. This provides a practical example of
how to use the repo argument and repository-related information.

Keeps your existing printf() calls in place.
This lets the users see the arguments which is given to the function.

This enhanced example provides a more complete illustration of
Adding a Git built-in command and use the repository argument.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index b463d42f63..ed6dcc1fc6 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -158,7 +158,7 @@ declaration for `cmd_pull`, and add a new line for `psuh` immediately before it,
 in order to keep the declarations alphabetically sorted:
 
 ----
-int cmd_psuh(int argc, const char **argv, const char *prefix);
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo);
 ----
 
 Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
@@ -174,7 +174,8 @@ 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)
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
+			const char *prefix UNUSED, struct repository *repo UNUSED)
 {
 	printf(_("Pony saying hello goes here.\n"));
 	return 0;
@@ -287,10 +288,14 @@ on the reference implementation linked at the top of this document.
 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, keeping
+Modify your `cmd_psuh` implementation to dump the args you're passed
+and removing the UNUSED macro from them, keeping
 existing `printf()` calls in place:
 
 ----
+int cmd_psuh(int argc, const char **argv, 
+			const char *prefix, struct repository *repo)
+{
 	int i;
 
 	...
@@ -305,6 +310,14 @@ existing `printf()` calls in place:
 	printf(_("Your current working directory:\n<top-level>%s%s\n"),
 	       prefix ? "/" : "", prefix ? prefix : "");
 
+	if (repo && repo->gitdir) {
+        printf(_("Git directory: %s\n"), repo->gitdir);
+    } else {
+        printf(_("No Git directory found.\n"));
+    }
+
+	...
+}
 ----
 
 Build and try it. As you may expect, there's pretty much just whatever we give
-- 
2.49.GIT

Re: [PATCH 3/4] Docs: Add cmd_psuh with repo and UNUSED removal

[Emily Shaffer]

On Wed, Apr 16, 2025 at 11:44:49AM +0530, K Jayatheerth wrote:
> 
> This commit improves the `cmd_psuh` documentation example by:
> 
> Correcting the function signature to include struct repository *repo.
> Makes the signature accurate and consistent with typical Git built-in
> commands.
> 
> Removing the `UNUSED` macros from the `cmd_psuh` function arguments
> (argc, argv, prefix, repo). This is done because the example now
> uses these arguments.
> 
> Showing how to access the repository's Git directory (repo->gitdir)
> within the cmd_psuh function. This provides a practical example of
> how to use the repo argument and repository-related information.
> 
> Keeps your existing printf() calls in place.
> This lets the users see the arguments which is given to the function.
> 
> This enhanced example provides a more complete illustration of
> Adding a Git built-in command and use the repository argument.

As I said for the prior patch, please revise the commit message; we
don't need the line by line description of what you're doing in the diff
below as we can read the diff :)

The important part is pointing out that the codebase has moved on to
require UNUSED and passing around a repository object, and that it's
interesting for newbies to see what's inside of `repo` in this learning
exercise.

> 
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index b463d42f63..ed6dcc1fc6 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -158,7 +158,7 @@ declaration for `cmd_pull`, and add a new line for `psuh` immediately before it,
>  in order to keep the declarations alphabetically sorted:
>  
>  ----
> -int cmd_psuh(int argc, const char **argv, const char *prefix);
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo);
>  ----
>  
>  Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
> @@ -174,7 +174,8 @@ 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)
> +int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
> +			const char *prefix UNUSED, struct repository *repo UNUSED)
>  {
>  	printf(_("Pony saying hello goes here.\n"));
>  	return 0;
> @@ -287,10 +288,14 @@ on the reference implementation linked at the top of this document.
>  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, keeping
> +Modify your `cmd_psuh` implementation to dump the args you're passed
> +and removing the UNUSED macro from them, keeping

"Modify ... and removing" mixes up tenses. Better to say,

Modify your `cmd_psuh` implementation to dump the args you're passed,
keeping existing `printf()` calls in place; because the args are now
used, remove the `UNUSED` macro from them:

>  existing `printf()` calls in place:
>  
>  ----
> +int cmd_psuh(int argc, const char **argv, 
> +			const char *prefix, struct repository *repo)
> +{
>  	int i;
>  
>  	...
> @@ -305,6 +310,14 @@ existing `printf()` calls in place:
>  	printf(_("Your current working directory:\n<top-level>%s%s\n"),
>  	       prefix ? "/" : "", prefix ? prefix : "");
>  
> +	if (repo && repo->gitdir) {
> +        printf(_("Git directory: %s\n"), repo->gitdir);
> +    } else {
> +        printf(_("No Git directory found.\n"));
> +    }

Your whitespace is wonky here, Git uses tabs, not spaces. Double check
it, please :)

> +
> +	...
> +}
>  ----
>  
>  Build and try it. As you may expect, there's pretty much just whatever we give
> -- 
> 2.49.GIT
> 
> 

[PATCH v2 1/3] docs: remove unused mentoring mailing list reference

[K Jayatheerth]

The git-mentoring group is being removed because new contributors
now approach the main mailing list directly and
almost always receive responses.
To reflect current practices and avoid confusion,
references to the unused mentoring list have been removed.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ca1d688c9b..ef190d8748 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
 The https://lore.kernel.org/git[archive] of this mailing list is
 available to view in a browser.
 
-==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
-
-This mailing list is targeted to new contributors and was created as a place to
-post questions and receive answers outside of the public eye of the main list.
-Veteran contributors who are especially interested in helping mentor newcomers
-are present on the list. In order to avoid search indexers, group membership is
-required to view messages; anyone can join and no approval is required.
-
 ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
 
 This IRC channel is for conversations between Git contributors. If someone is
-- 
2.49.GIT

[PATCH v2 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[K Jayatheerth]

The documentation previously omitted the UNUSED macro,
which often led to confusion for new contributors
when they encountered compiler warnings related to unused parameters.
This commit adds a brief explanation to help clarify its
purpose and common usage in the Git codebase.

Additionally, the function signature for cmd_psuh has been updated
to include the struct repository *repo argument,
aligning it with the standard pattern for built-in commands.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ef190d8748..f4320d8869 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -142,7 +142,15 @@ 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)
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
+----
+
+We will use the UNUSED macro to make sure we don't recieve compiler warnings
+for unused arguments from the function cmd_psuh.
+
+----
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
+	    const char *prefix UNUSED, struct repository *repo UNUSED)
 ----
 
 We'll also need to add the declaration of psuh; open up `builtin.h`, find the
@@ -150,7 +158,7 @@ declaration for `cmd_pull`, and add a new line for `psuh` immediately before it,
 in order to keep the declarations alphabetically sorted:
 
 ----
-int cmd_psuh(int argc, const char **argv, const char *prefix);
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo);
 ----
 
 Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
@@ -166,7 +174,8 @@ 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)
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
+	    const char *prefix UNUSED, struct repository *repo UNUSED)
 {
 	printf(_("Pony saying hello goes here.\n"));
 	return 0;
@@ -279,8 +288,9 @@ on the reference implementation linked at the top of this document.
 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, keeping
-existing `printf()` calls in place:
+Modify your `cmd_psuh` implementation to dump the args you're passed,
+keeping existing `printf()` calls in place; because the args are now
+used, remove the `UNUSED` macro from them:
 
 ----
 	int i;
-- 
2.49.GIT

Re: [PATCH v2 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> The documentation previously omitted the UNUSED macro,
> which often led to confusion for new contributors
> when they encountered compiler warnings related to unused parameters.

The above is not quite easy to reason about.  It is more like we
wrote this document, and then later tightened the default compiler
warnings for developer builds.  So "omitted" may technically be
correct, but it was more like "did not use it, because there was no
need".

    The sample program, as written, would not build for at least two
    reasons:

    - Since this document was first written, the calling convention
      to subcommand implementation has changed, and now cmd_psuh()
      needs to accept the third parameter, repository.

    - These days, compiler warning options for developers include
      one that detects and complains about unused parameters, so
      ones that are deliberately unused have to be marked as such.

After such observation on the status quo and description of the
problem you are going to solve, you give an order to the code base
to fix it, perhaps like:

    Update the old-style examples to adjust to the current
    practices, with explanations as needed.


To recap, the usual way to compose a log message of this project is
to

 - Give an observation on how the current system works in the
   present tense (so no need to say "Currently X is Y", or
   "Previously X was Y" to describe the state before your change;
   just "X is Y" is enough), and discuss what you perceive as a
   problem in it.

 - Propose a solution (optional---often, problem description
   trivially leads to an obvious solution in reader's minds).

 - Give commands to the codebase to "become like so".

in this order.

>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ef190d8748..f4320d8869 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -142,7 +142,15 @@ 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)
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
> +----
> +
> +We will use the UNUSED macro to make sure we don't recieve compiler warnings
> +for unused arguments from the function cmd_psuh.
> +----
> +int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
> +	    const char *prefix UNUSED, struct repository *repo UNUSED)
>  ----

I do not quite understand.  Why do we need a new one here?  Wouldn't
it be easier to read for a newcomer if you just give the last one
and explain what UNUSED are for?  Perhaps like

    ... matching the style and signature:

    ----
    int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
	         const char *prefix UNUSED, struct repository *repo UNUSED)
    ----

    A few things to note:

    * A subcommand implementation takes its command line arguments
      in `int argc` + `const char **argv`, like `main()` would

    * It also takes two extra parameters, `prefix` and `repo`.  What
      they mean will not be discussed until much later.

    * Because this first example will not use any of the parameters,
      your compiler will give warnings on unused parameters.  As the
      list of these four parameters is mandated by the API to add
      new built-in commands, you cannot omit them.  Instead, you add
      `UNUSED` to each of them to tell the compiler that you _know_
      you are not (yet) using it.

Take a special note on the last one.  There may be multiple ways to
squelch warnings, but it is worth telling your readers that use of
UNUSED is the right way.

I'll stop here for this patch.

Thanks.

Re: [PATCH v2 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
>
>> The documentation previously omitted the UNUSED macro,
>> which often led to confusion for new contributors
>> when they encountered compiler warnings related to unused parameters.
>
> The above is not quite easy to reason about.  It is more like we
> wrote this document, and then later tightened the default compiler
> warnings for developer builds.  So "omitted" may technically be
> correct, but it was more like "did not use it, because there was no
> need".
>
>     The sample program, as written, would not build for at least two
>     reasons:

"not" -> "no longer" to hint that the change in the environment,
not a bug in the document, is the reason of breakage.

>     - Since this document was first written, the calling convention
>       to subcommand implementation has changed, and now cmd_psuh()
>       needs to accept the third parameter, repository.

"third" -> "fourth"; I cannot count X-<.

[PATCH v3 0/3] Update MyFirstContribution.adoc to follow modern practices

[K Jayatheerth]

The following changes are made to MyFirstContribution.adoc 
to make the tutorial follow modern git practices.

The tutorial codes actually help newbies like myself to actually know what is going on 
in the source code as the Git source code is vast.

Therefore these patches.

K Jayatheerth (3):
  docs: remove unused mentoring mailing list reference
  docs: clarify cmd_psuh signature and explain UNUSED macro
  docs: replace git_config to repo_config

 Documentation/MyFirstContribution.adoc | 55 +++++++++++++++-----------
 1 file changed, 33 insertions(+), 22 deletions(-)

-- 
2.49.GIT

[PATCH v3 1/3] docs: remove unused mentoring mailing list reference

[K Jayatheerth]

The git-mentoring group was initially created to help newcomers
with their development itches. However, in practice,
most of their questions were already being addressed
directly on the mailing list, and contributors consistently
received helpful responses there.

Remove the mentoring group details from the Documentation.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ca1d688c9b..ef190d8748 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
 The https://lore.kernel.org/git[archive] of this mailing list is
 available to view in a browser.
 
-==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
-
-This mailing list is targeted to new contributors and was created as a place to
-post questions and receive answers outside of the public eye of the main list.
-Veteran contributors who are especially interested in helping mentor newcomers
-are present on the list. In order to avoid search indexers, group membership is
-required to view messages; anyone can join and no approval is required.
-
 ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
 
 This IRC channel is for conversations between Git contributors. If someone is
-- 
2.49.GIT

[PATCH v3 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[K Jayatheerth]

The sample program, as written, would no longer build for at least two
reasons:

    - Since this document was first written, the calling convention
      to subcommand implementation has changed, and now cmd_psuh()
      needs to accept the fourth parameter, repository.

    - These days, compiler warning options for developers include
      one that detects and complains about unused parameters, so
      ones that are deliberately unused have to be marked as such.

Update the old-style examples to adjust to the current
practices, with explanations as needed.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 28 +++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ef190d8748..da15d43d1f 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -142,15 +142,31 @@ 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)
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
+	     const char *prefix UNUSED, struct repository *repo UNUSED)
 ----
 
+A few things to note:
+
+* A subcommand implementation takes its command line arguments
+  in `int argc` + `const char **argv`, like `main()` would.
+
+* It also takes two extra parameters, `prefix` and `repo`. What
+  they mean will not be discussed until much later.
+
+* Because this first example will not use any of the parameters,
+  your compiler will give warnings on unused parameters. As the
+  list of these four parameters is mandated by the API to add
+  new built-in commands, you cannot omit them. Instead, you add
+  `UNUSED` to each of them to tell the compiler that you *know*
+  you are not (yet) using it.
+
 We'll also need to add the declaration of psuh; open up `builtin.h`, find the
 declaration for `cmd_pull`, and add a new line for `psuh` immediately before it,
 in order to keep the declarations alphabetically sorted:
 
 ----
-int cmd_psuh(int argc, const char **argv, const char *prefix);
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo);
 ----
 
 Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
@@ -166,7 +182,8 @@ 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)
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
+	     const char *prefix UNUSED, struct repository *repo UNUSED)
 {
 	printf(_("Pony saying hello goes here.\n"));
 	return 0;
@@ -279,8 +296,9 @@ on the reference implementation linked at the top of this document.
 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, keeping
-existing `printf()` calls in place:
+Modify your `cmd_psuh` implementation to dump the args you're passed,
+keeping existing `printf()` calls in place; because the args are now
+used, remove the `UNUSED` macro from them:
 
 ----
 	int i;
-- 
2.49.GIT

[PATCH v3 3/3] docs: replace git_config to repo_config

[K Jayatheerth]

Since this document was written, the built-in API has been
updated a few times, but the document was left stale.

Adjust to the current best practices by calling repo_config() on the
repository instance the subcommand implementation receives as a
parameter, instead of calling git_config() that used to be the
common practice.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index da15d43d1f..08a3137101 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -322,7 +322,8 @@ 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
+Add a line to `#include "config.h"` and `#include "repository.h"`.
+Then, add the following bits to the function body:
 function body:
 
 ----
@@ -330,18 +331,18 @@ function body:
 
 ...
 
-	git_config(git_default_config, NULL);
-	if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
+	repo_config(repo, git_default_config, NULL);
+	if (repo_config_get_string_tmp(repo, "user.name", &cfg_name))
 		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_tmp()` will look up
+`repo_config()` will grab the configuration from config files known to Git and
+apply standard precedence rules. `repo_config_get_string_tmp()` 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.adoc`.
+about how to use `repo_config()`) in `Documentation/technical/api-config.adoc`.
 
 You should see that the name printed matches the one you see when you run:
 
@@ -374,7 +375,7 @@ 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.
+to `repo_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`:
 
@@ -390,8 +391,8 @@ prepare it, and print its contents:
 
 ...
 
-	wt_status_prepare(the_repository, &status);
-	git_config(git_default_config, &status);
+	wt_status_prepare(repo, &status);
+	repo_config(repo, git_default_config, &status);
 
 ...
 
-- 
2.49.GIT

Re: [PATCH v3 3/3] docs: replace git_config to repo_config

[JAYATHEERTH K]

Whoops : )

I think I did V3 again

I think I should really start a new thread because this thread has 2
different sets of conversations going on
Will send v4 in a new thread

-Jayatheerth

[PATCH v2 3/3] docs: replace git_config to repo_config

[K Jayatheerth]

This change updates the example in cmd_psuh to use repo_config and
repo_config_get_string_tmp instead of the global git_config functions.

While git_config() accesses global configuration via the_repository,
using repo_config() makes use of the repo parameter passed to built-in commands.
This is the preferred pattern in the Git codebase,
as it respects repository-specific configuration (e.g., .git/config)
and avoids relying on global state.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index f4320d8869..8c2ca5c092 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -314,7 +314,8 @@ 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
+Add a line to `#include "config.h"` and `#include "repository.h"`.
+Then, add the following bits to the function body:
 function body:
 
 ----
@@ -322,18 +323,18 @@ function body:
 
 ...
 
-	git_config(git_default_config, NULL);
-	if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
+	repo_config(repo, git_default_config, NULL);
+	if (repo_config_get_string_tmp(repo, "user.name", &cfg_name))
 		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_tmp()` will look up
+`repo_config()` will grab the configuration from config files known to Git and
+apply standard precedence rules. `repo_config_get_string_tmp()` 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.adoc`.
+about how to use `repo_config()`) in `Documentation/technical/api-config.adoc`.
 
 You should see that the name printed matches the one you see when you run:
 
@@ -366,7 +367,7 @@ 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.
+to `repo_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`:
 
@@ -382,8 +383,8 @@ prepare it, and print its contents:
 
 ...
 
-	wt_status_prepare(the_repository, &status);
-	git_config(git_default_config, &status);
+	wt_status_prepare(repo, &status);
+	repo_config(repo, git_default_config, &status);
 
 ...
 
-- 
2.49.GIT

Re: [PATCH v2 3/3] docs: replace git_config to repo_config

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> This change updates the example in cmd_psuh to use repo_config and
> repo_config_get_string_tmp instead of the global git_config functions.
>
> While git_config() accesses global configuration via the_repository,
> using repo_config() makes use of the repo parameter passed to built-in commands.
> This is the preferred pattern in the Git codebase,
> as it respects repository-specific configuration (e.g., .git/config)
> and avoids relying on global state.

Again, do not start with "I did this, I did that".  The reason why
you needed to do such things is a lot more important.

    Since this document was written, the built-in API has been
    updated a few times, but the document was left stale.  Adjust to
    the current best practices by calling repo_config() on the
    repository instance the subcommand implementation receives as a
    parameter, instead of calling git_config() that used to be the
    common practice.

or something like that, perhaps.

Re: [PATCH v2 1/3] docs: remove unused mentoring mailing list reference

[Emily Shaffer]

On Sat, May 17, 2025 at 12:25:14AM +0530, K Jayatheerth wrote:
> 
> The git-mentoring group is being removed because new contributors
> now approach the main mailing list directly and
> almost always receive responses.
> To reflect current practices and avoid confusion,
> references to the unused mentoring list have been removed.
> 
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ca1d688c9b..ef190d8748 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
>  The https://lore.kernel.org/git[archive] of this mailing list is
>  available to view in a browser.
>  
> -==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
> -
> -This mailing list is targeted to new contributors and was created as a place to
> -post questions and receive answers outside of the public eye of the main list.
> -Veteran contributors who are especially interested in helping mentor newcomers
> -are present on the list. In order to avoid search indexers, group membership is
> -required to view messages; anyone can join and no approval is required.
> -
>  ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
>  
>  This IRC channel is for conversations between Git contributors. If someone is
> -- 
> 2.49.GIT

The contents of the series now look good to me, I have no problem with
them being merged as is.

By the way, when I look at your history on the mailing list I think this
is your first multi-patch series; I think when preparing v2 you forgot to
include --cover-letter in your arguments to `git format-patch`. On this
list it's more typical to always include a cover letter with multipatch
series, which you did with your initial version but forgot on this one.
:)

I'll leave it up to Junio whether this is acceptable to merge as-is or
whether he needs a cover letter, but all of v2 LGTM.

 - Emily

> 
> 

Re: [PATCH v2 1/3] docs: remove unused mentoring mailing list reference

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> The git-mentoring group is being removed because new contributors
> now approach the main mailing list directly and
> almost always receive responses.
> To reflect current practices and avoid confusion,
> references to the unused mentoring list have been removed.

We typically phrase the last line more like

	remove the reference to the unused mentoring group.

i.e., to give an order to the sources to "become like so".  Also,
the early part contrasted the mentoring group vs the main mailing
list, so "group" would probably be a less confusing reference.

Other than that, looks OK.  The actual change is also good.

> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ca1d688c9b..ef190d8748 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
>  The https://lore.kernel.org/git[archive] of this mailing list is
>  available to view in a browser.
>  
> -==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
> -
> -This mailing list is targeted to new contributors and was created as a place to
> -post questions and receive answers outside of the public eye of the main list.
> -Veteran contributors who are especially interested in helping mentor newcomers
> -are present on the list. In order to avoid search indexers, group membership is
> -required to view messages; anyone can join and no approval is required.
> -
>  ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
>  
>  This IRC channel is for conversations between Git contributors. If someone is

[PATCH v3 0/3] Update MyFirstContribution.adoc to follow modern practices

[K Jayatheerth]

The following changes are made to MyFirstContribution.adoc 
to make the tutorial follow modern git practices.

The tutorial codes actually help newbies like myself to actually know what is going on 
in the source code as the Git source code is vast.

Therefore these patches.


K Jayatheerth (3):
  docs: remove unused mentoring mailing list reference
  docs: clarify cmd_psuh signature and explain UNUSED macro
  docs: replace git_config to repo_config

 Documentation/MyFirstContribution.adoc | 47 ++++++++++++++------------
 1 file changed, 25 insertions(+), 22 deletions(-)

-- 
2.49.GIT

[PATCH v3 1/3] docs: remove unused mentoring mailing list reference

[K Jayatheerth]

The git-mentoring group is being removed because new contributors
now approach the main mailing list directly and
almost always receive responses.
To reflect current practices and avoid confusion,
remove the reference to the unused mentoring group.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ca1d688c9b..ef190d8748 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -40,14 +40,6 @@ the list by sending an email to <git+subscribe@vger.kernel.org>
 The https://lore.kernel.org/git[archive] of this mailing list is
 available to view in a browser.
 
-==== https://groups.google.com/forum/#!forum/git-mentoring[git-mentoring@googlegroups.com]
-
-This mailing list is targeted to new contributors and was created as a place to
-post questions and receive answers outside of the public eye of the main list.
-Veteran contributors who are especially interested in helping mentor newcomers
-are present on the list. In order to avoid search indexers, group membership is
-required to view messages; anyone can join and no approval is required.
-
 ==== https://web.libera.chat/#git-devel[#git-devel] on Libera Chat
 
 This IRC channel is for conversations between Git contributors. If someone is
-- 
2.49.GIT

[PATCH v3 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro

[K Jayatheerth]

The documentation previously omitted the UNUSED macro,
which often led to confusion for new contributors
when they encountered compiler warnings related to unused parameters.
This commit adds a brief explanation to help clarify its
purpose and common usage in the Git codebase.

Additionally, the function signature for cmd_psuh has been updated
to include the struct repository *repo argument,
align it with standard practices.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ef190d8748..f4320d8869 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -142,7 +142,15 @@ 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)
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
+----
+
+We will use the UNUSED macro to make sure we don't recieve compiler warnings
+for unused arguments from the function cmd_psuh.
+
+----
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED,
+	    const char *prefix UNUSED, struct repository *repo UNUSED)
 ----
 
 We'll also need to add the declaration of psuh; open up `builtin.h`, find the
@@ -150,7 +158,7 @@ declaration for `cmd_pull`, and add a new line for `psuh` immediately before it,
 in order to keep the declarations alphabetically sorted:
 
 ----
-int cmd_psuh(int argc, const char **argv, const char *prefix);
+int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo);
 ----
 
 Be sure to `#include "builtin.h"` in your `psuh.c`. You'll also need to
@@ -166,7 +174,8 @@ 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)
+int cmd_psuh(int argc UNUSED, const char **argv UNUSED, 
+	    const char *prefix UNUSED, struct repository *repo UNUSED)
 {
 	printf(_("Pony saying hello goes here.\n"));
 	return 0;
@@ -279,8 +288,9 @@ on the reference implementation linked at the top of this document.
 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, keeping
-existing `printf()` calls in place:
+Modify your `cmd_psuh` implementation to dump the args you're passed,
+keeping existing `printf()` calls in place; because the args are now
+used, remove the `UNUSED` macro from them:
 
 ----
 	int i;
-- 
2.49.GIT

[PATCH v3 3/3] docs: replace git_config to repo_config

[K Jayatheerth]

This change updates the example in cmd_psuh to use repo_config and
repo_config_get_string_tmp instead of the global git_config functions.

While git_config() accesses global configuration via the_repository,
using repo_config() makes use of the repo parameter passed to built-in commands.
This is the preferred pattern in the Git codebase,
respect repository-specific configuration (e.g., .git/config)
and avoid relying on global state.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index f4320d8869..8c2ca5c092 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -314,7 +314,8 @@ 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
+Add a line to `#include "config.h"` and `#include "repository.h"`.
+Then, add the following bits to the function body:
 function body:
 
 ----
@@ -322,18 +323,18 @@ function body:
 
 ...
 
-	git_config(git_default_config, NULL);
-	if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
+	repo_config(repo, git_default_config, NULL);
+	if (repo_config_get_string_tmp(repo, "user.name", &cfg_name))
 		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_tmp()` will look up
+`repo_config()` will grab the configuration from config files known to Git and
+apply standard precedence rules. `repo_config_get_string_tmp()` 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.adoc`.
+about how to use `repo_config()`) in `Documentation/technical/api-config.adoc`.
 
 You should see that the name printed matches the one you see when you run:
 
@@ -366,7 +367,7 @@ 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.
+to `repo_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`:
 
@@ -382,8 +383,8 @@ prepare it, and print its contents:
 
 ...
 
-	wt_status_prepare(the_repository, &status);
-	git_config(git_default_config, &status);
+	wt_status_prepare(repo, &status);
+	repo_config(repo, git_default_config, &status);
 
 ...
 
-- 
2.49.GIT

[PATCH 4/4] cmd_psuh: Prefer repo_config for config lookup

[K Jayatheerth]

This commit updates cmd_psuh to use repo_config and
repo_config_get_string_tmp for retrieving the user.name config
variable. This is a more robust and correct approach than using the
global git_config functions because:

git_config uses the global configuration, ignoring any
repository-specific settings (e.g., in .git/config). repo_config
loads the configuration specific to the repository,
ensuring that the correct settings are used.

repo_config_get_string_tmp retrieves configuration values
relative to the repository, respecting any local overrides.

This change ensures that cmd_psuh correctly reads the
user.name setting that applies to the current repository,
rather than relying on globalsettings that might be
incorrect or misleading. It also demonstrates the correct way
to access repository-specific configuration within Git commands.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/MyFirstContribution.adoc | 52 ++++++++++++++++++--------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index ed6dcc1fc6..688240ce8b 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -325,26 +325,48 @@ 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:
+Add a line to `#include "config.h"` and `#include "repository.h"`. 
+Then, add the following bits to the function body:
 
 ----
-	const char *cfg_name;
+#include "builtin.h"
+#include "gettext.h"
+#include "config.h"
+#include "repository.h"  
 
-...
+int cmd_psuh(int argc, const char **argv, 
+			const char *prefix, struct repository *repo)
+{
+    const char *cfg_name;
+
+    printf(Q_("Your args (there is %d):\n",
+              "Your args (there are %d):\n",
+              argc),
+           argc);
 
-	git_config(git_default_config, NULL);
-	if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
-		printf(_("No name is found in config\n"));
-	else
-		printf(_("Your name: %s\n"), cfg_name);
+    for (int 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 : "");
+
+    repo_config(repo, git_default_config, NULL);
+
+    if (repo_config_get_string_tmp(repo, "user.name", &cfg_name))
+        printf(_("No name is found in config\n"));
+    else
+        printf(_("Your name: %s\n"), cfg_name);
+
+    return 0;
+}
 ----
 
-`git_config()` will grab the configuration from config files known to Git and
-apply standard precedence rules. `git_config_get_string_tmp()` will look up
+`repo_config()` will grab the configuration from config files known to Git and
+apply standard precedence rules. `repo_config_get_string_tmp()` 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.adoc`.
+about how to use `repo_config()`) in `Documentation/technical/api-config.adoc`.
 
 You should see that the name printed matches the one you see when you run:
 
@@ -377,7 +399,7 @@ 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.
+to `repo_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`:
 
@@ -393,8 +415,8 @@ prepare it, and print its contents:
 
 ...
 
-	wt_status_prepare(the_repository, &status);
-	git_config(git_default_config, &status);
+	wt_status_prepare(repo, &status);
+	repo_config(repo, git_default_config, &status);
 
 ...
 
-- 
2.49.GIT

Re: [PATCH 4/4] cmd_psuh: Prefer repo_config for config lookup

[Emily Shaffer]

On Wed, Apr 16, 2025 at 11:44:50AM +0530, K Jayatheerth wrote:
> 
> This commit updates cmd_psuh to use repo_config and
> repo_config_get_string_tmp for retrieving the user.name config
> variable. This is a more robust and correct approach than using the
> global git_config functions because:
> 
> git_config uses the global configuration, ignoring any
> repository-specific settings (e.g., in .git/config). repo_config
> loads the configuration specific to the repository,
> ensuring that the correct settings are used.
> 
> repo_config_get_string_tmp retrieves configuration values
> relative to the repository, respecting any local overrides.
> 
> This change ensures that cmd_psuh correctly reads the
> user.name setting that applies to the current repository,
> rather than relying on globalsettings that might be
> incorrect or misleading. It also demonstrates the correct way
> to access repository-specific configuration within Git commands.

This commit message is a really good start! I like that you're pointing
out there's a real bug in using global config and ignoring repo-local
config. Although I'm not sure that it's actually accurate... but it is
true that git_config() is now an outdated way to access config, since it
assumes the_repository instead of passing the `repo` variable that was
provided to cmd_psuh.

> 
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  Documentation/MyFirstContribution.adoc | 52 ++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
> index ed6dcc1fc6..688240ce8b 100644
> --- a/Documentation/MyFirstContribution.adoc
> +++ b/Documentation/MyFirstContribution.adoc
> @@ -325,26 +325,48 @@ 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:
> +Add a line to `#include "config.h"` and `#include "repository.h"`. 
> +Then, add the following bits to the function body:
>  
>  ----
> -	const char *cfg_name;
> +#include "builtin.h"
> +#include "gettext.h"
> +#include "config.h"
> +#include "repository.h"  
>  
> -...
> +int cmd_psuh(int argc, const char **argv, 
> +			const char *prefix, struct repository *repo)
> +{
> +    const char *cfg_name;
> +
> +    printf(Q_("Your args (there is %d):\n",
> +              "Your args (there are %d):\n",
> +              argc),
> +           argc);
>  
> -	git_config(git_default_config, NULL);
> -	if (git_config_get_string_tmp("user.name", &cfg_name) > 0)
> -		printf(_("No name is found in config\n"));
> -	else
> -		printf(_("Your name: %s\n"), cfg_name);
> +    for (int 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 : "");
> +
> +    repo_config(repo, git_default_config, NULL);
> +
> +    if (repo_config_get_string_tmp(repo, "user.name", &cfg_name))
> +        printf(_("No name is found in config\n"));
> +    else
> +        printf(_("Your name: %s\n"), cfg_name);
> +
> +    return 0;
> +}

I'd prefer to see this stick to the prior formula of including only
small chunks of the function, rather than a full function you can copy
and paste. Because this is a tutorial, and the goal is for learners to
understand each section of code as they add it, not just for them to
paste it into their editor and hit run.

So, I don't think it's necessary for you to add the rest of the function
here in the process of switching to repo_config from git_config.


Generally, I find the changes to update the code snippets
unobjectionable and don't have a problem with the added prose
beyond a couple nits. But as I assume you sent this series as a way to
learn more about the codebase, definitely please revisit your commit
messages to align their style with the rest of the codebase.

But I think with the stuff I called out taken into account in v2, this
series is good. Thanks for the effort to update it. I'd also like to
update github.com/nasamuffin/git/tree/psuh once this series lands, if
you can point me to a branch of yours with the sample code I can pull
from :)

(Or, as we discussed when I sent this doc in the first place, does it
make sense for a branch with the sample code to be maintained
only-best-effort on git/git itself?)

 - Emily

>
>  ----
>  
> -`git_config()` will grab the configuration from config files known to Git and
> -apply standard precedence rules. `git_config_get_string_tmp()` will look up
> +`repo_config()` will grab the configuration from config files known to Git and
> +apply standard precedence rules. `repo_config_get_string_tmp()` 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.adoc`.
> +about how to use `repo_config()`) in `Documentation/technical/api-config.adoc`.
>  
>  You should see that the name printed matches the one you see when you run:
>  
> @@ -377,7 +399,7 @@ 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.
> +to `repo_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`:
>  
> @@ -393,8 +415,8 @@ prepare it, and print its contents:
>  
>  ...
>  
> -	wt_status_prepare(the_repository, &status);
> -	git_config(git_default_config, &status);
> +	wt_status_prepare(repo, &status);
> +	repo_config(repo, git_default_config, &status);
>  
>  ...
>  
> -- 
> 2.49.GIT
> 
> 

Re: [PATCH 4/4] cmd_psuh: Prefer repo_config for config lookup

[JAYATHEERTH K]

So, I've consolidated these patches in only 3 rather than 4,
the UNUSED fix patch was actually delving into 2 patches so I fixed
that in a single one.

Also the commit messages, I gotta admit I was given the same feedback from Junio
But these were actually written before that, Apologies I should've checked once.


> I'd prefer to see this stick to the prior formula of including only
> small chunks of the function, rather than a full function you can copy
> and paste. Because this is a tutorial, and the goal is for learners to
> understand each section of code as they add it, not just for them to
> paste it into their editor and hit run.
>

Yes I've done this. I've just changed git_config to repo_config
without changing the other parts.

> So, I don't think it's necessary for you to add the rest of the function
> here in the process of switching to repo_config from git_config.
>
>
> Generally, I find the changes to update the code snippets
> unobjectionable and don't have a problem with the added prose
> beyond a couple nits. But as I assume you sent this series as a way to
> learn more about the codebase, definitely please revisit your commit
> messages to align their style with the rest of the codebase.
>
> But I think with the stuff I called out taken into account in v2, this
> series is good. Thanks for the effort to update it. I'd also like to
> update github.com/nasamuffin/git/tree/psuh once this series lands, if
> you can point me to a branch of yours with the sample code I can pull
> from :)

Surely, I will write the builtin function with a test case and make
sure it compiles.

> (Or, as we discussed when I sent this doc in the first place, does it
> make sense for a branch with the sample code to be maintained
> only-best-effort on git/git itself?)
>

I think it's fine both ways as long as we link it to the proper place
in the docs.

>  - Emily
>
> >
> >  ----
...

In the new series I tried to keep commit messages as good as I could
without naming every line I edited and just keeping it to why instead of what.

Thank you,

-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

    To: git@vger.kernel.org
    Cc: Junio C Hamano <gitster@pobox.com>, jayatheerthkulkarni2005@gmail.com

Pick a better reviewer than me if you want your patches reviewed
more quickly, perhaps?  I had only 3 patches to the file in the past
3 years, while there are others who had their hands to the file with
more changes.  "git shortlog --no-merges --since=3.years" is your
friend ;-)

> A series of patches extending my GSOC microproject
> Fixing the documentation MyFirstContribution.adoc
>
> Updated various commits since the last patches sent.
>
> Feedbacks taken to write detailed commit messages
>
> K Jayatheerth (4):
>   Remove unused git-mentoring mailing list
>   Docs: Correct cmd_psuh and Explain UNUSED macro
>   Docs: Add cmd_psuh with repo and UNUSED removal
>   cmd_psuh: Prefer repo_config for config lookup
>
>  Documentation/MyFirstContribution.adoc | 89 ++++++++++++++++++--------
>  1 file changed, 62 insertions(+), 27 deletions(-)

Re: [PATCH 0/4] update MyFirstContribution with current code base

[JAYATHEERTH K]

On Wed, Apr 16, 2025 at 7:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
>
>     To: git@vger.kernel.org
>     Cc: Junio C Hamano <gitster@pobox.com>, jayatheerthkulkarni2005@gmail.com
>
> Pick a better reviewer than me if you want your patches reviewed
> more quickly, perhaps?  I had only 3 patches to the file in the past
> 3 years, while there are others who had their hands to the file with
> more changes.  "git shortlog --no-merges --since=3.years" is your
> friend ;-)
>
Umm I got no issues with slow reviews too, but I also ran the command
and I think the most amounts of commits for this file looks something like this

git shortlog --no-merges --since=6.years -- Documentation/MyFirstContributi
on.adoc
Jean-Noël Avila (1):
      doc: add a blank line around block delimiters

K Jayatheerth (4):
      Remove unused git-mentoring mailing list
      Docs: Correct cmd_psuh and Explain UNUSED macro
      Docs: Add cmd_psuh with repo and UNUSED removal
      cmd_psuh: Prefer repo_config for config lookup

Todd Zullinger (1):
      MyFirstContribution: *.txt -> *.adoc fixes

brian m. carlson (1):
      doc: use .adoc extension for AsciiDoc files


If you can suggest someone, I could CC them in this thread.

Thank you,
-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[JAYATHEERTH K]

Hey people from Git,

Looks like this might have gotten lost in the void.

Would appreciate it if someone could take a look at this series of patches.
These are documentation-only changes and should not have any effect on
the actual source code.

Thanks in advance!

-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[Junio C Hamano]

JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:

> git shortlog --no-merges --since=6.years -- Documentation/MyFirstContributi
> on.adoc

That would not work; due to mass rename, you'd need

  $ git shortlog --no-merges -- Documentation/MyFirstContribution.{adoc,txt}

or perhaps

  $ git log --no-merges --follow Documentation/MyFirstContribution.adoc |
    git shortlog

or something like that.

Re: [PATCH 0/4] update MyFirstContribution with current code base

[JAYATHEERTH K]

On Wed, May 14, 2025 at 6:18 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:
>
> > git shortlog --no-merges --since=6.years -- Documentation/MyFirstContributi
> > on.adoc
>
> That would not work; due to mass rename, you'd need
>
>   $ git shortlog --no-merges -- Documentation/MyFirstContribution.{adoc,txt}
>
> or perhaps
>
>   $ git log --no-merges --follow Documentation/MyFirstContribution.adoc |
>     git shortlog
>
> or something like that.
>

Oh I totally forgot the extension was changed to adoc
Thanks for pointing it out

As far as I see

Emily Shaffer (9)
Philippe Blain (5)

Have the two highest logs
I think Emily is not active in Git
So for now I will cc Philippe

Thank you,

-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[Emily Shaffer]

On Wed, May 14, 2025 at 6:07 AM JAYATHEERTH K
<jayatheerthkulkarni2005@gmail.com> wrote:
> As far as I see
>
> Emily Shaffer (9)
> Philippe Blain (5)
>
> Have the two highest logs
> I think Emily is not active in Git

Mostly I lurk these days :) I do still keep an eye on the list. Will
happily take a look at your series tomorrow, I'm out of time for
today. But per what I mention below, if you don't hear from me, please
don't feel blocked by the review, as I think the MyFirstContribution
doc is comfortably maintained by the whole project by now.

> So for now I will cc Philippe

For what it's worth, I don't think it is harmful to CC people even if
they will be inactive. CCing someone is not necessarily the same thing
as saying that person needs to approve your code change, right? So I
don't see the harm in CCing with low expectations - in fact, in my
case it would help make the email stand out, so you'd be more likely
to get a review from me (I missed this thread going by initially).


 - Emily

Re: [PATCH 0/4] update MyFirstContribution with current code base

[JAYATHEERTH K]

On Fri, May 16, 2025 at 4:09 AM Emily Shaffer <nasamuffin@google.com> wrote:

> Mostly I lurk these days :) I do still keep an eye on the list. Will
> happily take a look at your series tomorrow, I'm out of time for
> today. But per what I mention below, if you don't hear from me, please
> don't feel blocked by the review, as I think the MyFirstContribution
> doc is comfortably maintained by the whole project by now.
>

Understood!! thanks for letting me know

> > So for now I will cc Philippe
>
> For what it's worth, I don't think it is harmful to CC people even if
> they will be inactive. CCing someone is not necessarily the same thing
> as saying that person needs to approve your code change, right? So I
> don't see the harm in CCing with low expectations - in fact, in my
> case it would help make the email stand out, so you'd be more likely
> to get a review from me (I missed this thread going by initially).
>
>

Oh, ok I will keep that in mind next time.

>  - Emily

Thank you,

-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[Junio C Hamano]

JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:

> On Fri, May 16, 2025 at 4:09 AM Emily Shaffer <nasamuffin@google.com> wrote:
>
>> Mostly I lurk these days :) I do still keep an eye on the list. Will
>> happily take a look at your series tomorrow, I'm out of time for
>> today. But per what I mention below, if you don't hear from me, please
>> don't feel blocked by the review, as I think the MyFirstContribution
>> doc is comfortably maintained by the whole project by now.
>>
>
> Understood!! thanks for letting me know
>
>> > So for now I will cc Philippe
>>
>> For what it's worth, I don't think it is harmful to CC people even if
>> they will be inactive. CCing someone is not necessarily the same thing
>> as saying that person needs to approve your code change, right? So I
>> don't see the harm in CCing with low expectations - in fact, in my
>> case it would help make the email stand out, so you'd be more likely
>> to get a review from me (I missed this thread going by initially).
>>
>>
>
> Oh, ok I will keep that in mind next time.
>
>>  - Emily
>
> Thank you,

Thanks for a pleasant conversation; others can also learn from this
exchange, hopefully.  In Documentation/SubmittingPatches we have
"Choosing your reviewers" section lacks anything more concrete than
"who are involved in the area you are touching", and those who use
common sense may say, just like you did, "ah, most of the text I am
replacing was written N years ago by person X, whom I no longer see
on the list very often" and decide to omit it.  Perhaps we would
want to enhance the text there somewhat?  I dunno.

Since there were discussions on contrib/contacts recently (a few of
the participants there added to CC), I tried it and unfortunately I
was not very impressed by its output [*].

After applying the four patches on top of 'master', you'd run the
tool like so:

    $ contrib/contacts/git-contacts master..
    Jonathan Nieder <jrnieder@gmail.com>
    Jacob Stopak <jacob@initialcommit.io>
    Jeff King <peff@peff.net>
    Jean-Noël Avila <jn.avila@free.fr>
    Emily Shaffer <nasamuffin@google.com>
    Atharva Raykar <raykar.ath@gmail.com>
    Junio C Hamano <gitster@pobox.com>
    Todd Zullinger <tmz@pobox.com>
    Kyle Lippincott <spectral@google.com>

The tool gave output in a different order every time it was run.  It
wasn't obvious what the ordering meant.

By looking at its source, I can tell that the names and addresses
are collected from trailers like reported-by, which are counted with
the same importance as the authorship, that the reason why the
output is different each time it is run is due to use of keys %hash
in a Perl script, etc., but counting sign-off would mean that I'd be
summoned for each and every change related in this project, which
would not be very productive use of everybody's time.

And it of course is not clear who are still active in the recent
past and why the name was in the list (it would not be as productive
to ask for a review from somebody who was listed for reporting many
problems in the area affected by the proposed patch than those who
wrote the original) from this output.  There may want an "explain"
mode that lets you feed a patch and get observations like:

    The majority of lines you are touching haven't changed much
    since person X wrote commit W 5 years ago, and the text turned
    into current shape with contributions by person Y and Z.  Here
    are the URLs into the lore archive for the discussion that you
    can see how X, Y, and Z participated in the original before you
    touched.  You may also want to look at commit V and U as well.

    Last time we saw person X, Y, and Z on the list were ..., here
    are the URLs into the lore archive.
    
Perhaps some AI minded folks can write such a service for us ;-)?


[Footnote]

 * I didn't try other alternatives which I didn't have, and the
   other thread there was a mention of "git related" with "seems
   like rather more work".

   cf. https://lore.kernel.org/git/aBr9bwNQ1J46NNXI@pks.im/

Re: [PATCH 0/4] update MyFirstContribution with current code base

[JAYATHEERTH K]

On Fri, May 16, 2025 at 9:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:
>
> > On Fri, May 16, 2025 at 4:09 AM Emily Shaffer <nasamuffin@google.com> wrote:
> >
> >> Mostly I lurk these days :) I do still keep an eye on the list. Will
> >> happily take a look at your series tomorrow, I'm out of time for
> >> today. But per what I mention below, if you don't hear from me, please
> >> don't feel blocked by the review, as I think the MyFirstContribution
> >> doc is comfortably maintained by the whole project by now.
> >>
> >
> > Understood!! thanks for letting me know
> >
> >> > So for now I will cc Philippe
> >>
> >> For what it's worth, I don't think it is harmful to CC people even if
> >> they will be inactive. CCing someone is not necessarily the same thing
> >> as saying that person needs to approve your code change, right? So I
> >> don't see the harm in CCing with low expectations - in fact, in my
> >> case it would help make the email stand out, so you'd be more likely
> >> to get a review from me (I missed this thread going by initially).
> >>
> >>
> >
> > Oh, ok I will keep that in mind next time.
> >
> >>  - Emily
> >
> > Thank you,
>
> Thanks for a pleasant conversation; others can also learn from this
> exchange, hopefully.  In Documentation/SubmittingPatches we have
> "Choosing your reviewers" section lacks anything more concrete than
> "who are involved in the area you are touching", and those who use
> common sense may say, just like you did, "ah, most of the text I am
> replacing was written N years ago by person X, whom I no longer see
> on the list very often" and decide to omit it.  Perhaps we would
> want to enhance the text there somewhat?  I dunno.
>

Agreed even a single practical example in the "Choosing your
reviewers" section of SubmittingPatches could guide contributors
better.
I'd be happy to draft a patch that adds such a line, based on this
thread’s discussion.

> Since there were discussions on contrib/contacts recently (a few of
> the participants there added to CC), I tried it and unfortunately I
> was not very impressed by its output [*].
>
> After applying the four patches on top of 'master', you'd run the
> tool like so:
>
>     $ contrib/contacts/git-contacts master..
>     Jonathan Nieder <jrnieder@gmail.com>
>     Jacob Stopak <jacob@initialcommit.io>
>     Jeff King <peff@peff.net>
>     Jean-Noël Avila <jn.avila@free.fr>
>     Emily Shaffer <nasamuffin@google.com>
>     Atharva Raykar <raykar.ath@gmail.com>
>     Junio C Hamano <gitster@pobox.com>
>     Todd Zullinger <tmz@pobox.com>
>     Kyle Lippincott <spectral@google.com>
>
> The tool gave output in a different order every time it was run.  It
> wasn't obvious what the ordering meant.
>
> By looking at its source, I can tell that the names and addresses
> are collected from trailers like reported-by, which are counted with
> the same importance as the authorship, that the reason why the
> output is different each time it is run is due to use of keys %hash
> in a Perl script, etc., but counting sign-off would mean that I'd be
> summoned for each and every change related in this project, which
> would not be very productive use of everybody's time.
>

Agreed, but I don't know if there are any projects where there are no
authorship names
and direct commit details.
Or maybe there are two commits where it must create more confusion.

> And it of course is not clear who are still active in the recent
> past and why the name was in the list (it would not be as productive
> to ask for a review from somebody who was listed for reporting many
> problems in the area affected by the proposed patch than those who
> wrote the original) from this output.  There may want an "explain"
> mode that lets you feed a patch and get observations like:
>
>     The majority of lines you are touching haven't changed much
>     since person X wrote commit W 5 years ago, and the text turned
>     into current shape with contributions by person Y and Z.  Here
>     are the URLs into the lore archive for the discussion that you
>     can see how X, Y, and Z participated in the original before you
>     touched.  You may also want to look at commit V and U as well.
>
>     Last time we saw person X, Y, and Z on the list were ..., here
>     are the URLs into the lore archive.
>
> Perhaps some AI minded folks can write such a service for us ;-)?
>

If we're talking about AI approaches, I do think this could be
feasible with LLMs. I imagine a pipeline where:
A patch is parsed and matched to the line-level history (via git blame
or log -L)

The commit history is summarized to extract contributor roles
Activity is cross-checked on lore.kernel.org
An LLM generates human-readable explanations with references and
confidence indicators

Of course, the risk of hallucinations is real but with a properly
curated context (e.g., logs and emails as input, strict templates), I
think we can keep it grounded.
I'd like to prototype such a tool and would value the list's feedback
on this idea.

Also I think this idea would only make sense as a seperated solution
and not adding in Git
because it would cost a lot of compute to run LLMs locally, or perhaps like
email the way we add config on the before hand, (if we are combining
with git) giving people an option to add an API to their LLM would
work
But this is just a vague idea.

Thanks again and truly find this thread constructive.


>
> [Footnote]
>
>  * I didn't try other alternatives which I didn't have, and the
>    other thread there was a mention of "git related" with "seems
>    like rather more work".
>
>    cf. https://lore.kernel.org/git/aBr9bwNQ1J46NNXI@pks.im/



-Jayatheerth

Re: [PATCH 0/4] update MyFirstContribution with current code base

[D. Ben Knoble]

On Fri, May 16, 2025 at 12:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:
>
> > On Fri, May 16, 2025 at 4:09 AM Emily Shaffer <nasamuffin@google.com> wrote:
> >
> >> Mostly I lurk these days :) I do still keep an eye on the list. Will
> >> happily take a look at your series tomorrow, I'm out of time for
> >> today. But per what I mention below, if you don't hear from me, please
> >> don't feel blocked by the review, as I think the MyFirstContribution
> >> doc is comfortably maintained by the whole project by now.
> >>
> >
> > Understood!! thanks for letting me know
> >
> >> > So for now I will cc Philippe
> >>
> >> For what it's worth, I don't think it is harmful to CC people even if
> >> they will be inactive. CCing someone is not necessarily the same thing
> >> as saying that person needs to approve your code change, right? So I
> >> don't see the harm in CCing with low expectations - in fact, in my
> >> case it would help make the email stand out, so you'd be more likely
> >> to get a review from me (I missed this thread going by initially).
> >>
> >>
> >
> > Oh, ok I will keep that in mind next time.
> >
> >>  - Emily
> >
> > Thank you,
>
> Thanks for a pleasant conversation; others can also learn from this
> exchange, hopefully.  In Documentation/SubmittingPatches we have
> "Choosing your reviewers" section lacks anything more concrete than
> "who are involved in the area you are touching", and those who use
> common sense may say, just like you did, "ah, most of the text I am
> replacing was written N years ago by person X, whom I no longer see
> on the list very often" and decide to omit it.  Perhaps we would
> want to enhance the text there somewhat?  I dunno.
>
> Since there were discussions on contrib/contacts recently (a few of
> the participants there added to CC), I tried it and unfortunately I
> was not very impressed by its output [*].
>
> After applying the four patches on top of 'master', you'd run the
> tool like so:
>
>     $ contrib/contacts/git-contacts master..
>     Jonathan Nieder <jrnieder@gmail.com>
>     Jacob Stopak <jacob@initialcommit.io>
>     Jeff King <peff@peff.net>
>     Jean-Noël Avila <jn.avila@free.fr>
>     Emily Shaffer <nasamuffin@google.com>
>     Atharva Raykar <raykar.ath@gmail.com>
>     Junio C Hamano <gitster@pobox.com>
>     Todd Zullinger <tmz@pobox.com>
>     Kyle Lippincott <spectral@google.com>
>
> The tool gave output in a different order every time it was run.  It
> wasn't obvious what the ordering meant.
>
> By looking at its source, I can tell that the names and addresses
> are collected from trailers like reported-by, which are counted with
> the same importance as the authorship, that the reason why the
> output is different each time it is run is due to use of keys %hash
> in a Perl script, etc., but counting sign-off would mean that I'd be
> summoned for each and every change related in this project, which
> would not be very productive use of everybody's time.
>
> And it of course is not clear who are still active in the recent
> past and why the name was in the list (it would not be as productive
> to ask for a review from somebody who was listed for reporting many
> problems in the area affected by the proposed patch than those who
> wrote the original) from this output.  There may want an "explain"
> mode that lets you feed a patch and get observations like:
>
>     The majority of lines you are touching haven't changed much
>     since person X wrote commit W 5 years ago, and the text turned
>     into current shape with contributions by person Y and Z.  Here
>     are the URLs into the lore archive for the discussion that you
>     can see how X, Y, and Z participated in the original before you
>     touched.  You may also want to look at commit V and U as well.
>
>     Last time we saw person X, Y, and Z on the list were ..., here
>     are the URLs into the lore archive.
>
> Perhaps some AI minded folks can write such a service for us ;-)?
>
>
> [Footnote]
>
>  * I didn't try other alternatives which I didn't have, and the
>    other thread there was a mention of "git related" with "seems
>    like rather more work".
>
>    cf. https://lore.kernel.org/git/aBr9bwNQ1J46NNXI@pks.im/
>

I sometimes use another alternative "git overwritten" [1] which counts
blame information instead. I've not tried to modify it to be
compatible with contacts, though.

For example, after applying the initial version of patches (which had
whitespace errors, btw; didn't check the latest version) on 7a1d2bd0a5
(Merge branch 'master' of https://github.com/j6t/gitk, 2025-05-09),
"git overwritten" shows

  10 76644e3268 (documentation: add tutorial for first contribution,
2019-05-17) by Emily Shaffer
   5 a2dc43414c (MyFirstContribution: rephrase contact info,
2020-02-13) by Emily Shaffer
   3 4bb4fd4290 (MyFirstContribution: add avenues for getting help,
2020-01-24) by Emily Shaffer
   2 9a53219f69 (config: drop git_config_get_string_const(),
2020-08-17) by Jeff King
   2 2656fb16dd (doc: add some nit fixes to MyFirstContribution,
2019-05-29) by Emily Shaffer
   1 8b4b41aefb (MyFirstContribution: *.txt -> *.adoc fixes,
2025-03-03) by Todd Zullinger

[1]: https://github.com/benknoble/Dotfiles/blob/master/links/bin/git-overwritten

-- 
D. Ben Knoble

Re: [PATCH 0/4] update MyFirstContribution with current code base

[Junio C Hamano]

JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:

> On Fri, May 16, 2025 at 4:09 AM Emily Shaffer <nasamuffin@google.com> wrote:
>
>> Mostly I lurk these days :) I do still keep an eye on the list. Will
>> happily take a look at your series tomorrow, I'm out of time for
>> today. But per what I mention below, if you don't hear from me, please
>> don't feel blocked by the review, as I think the MyFirstContribution
>> doc is comfortably maintained by the whole project by now.
>>
>
> Understood!! thanks for letting me know
>
>> > So for now I will cc Philippe
>>
>> For what it's worth, I don't think it is harmful to CC people even if
>> they will be inactive. CCing someone is not necessarily the same thing
>> as saying that person needs to approve your code change, right? So I
>> don't see the harm in CCing with low expectations - in fact, in my
>> case it would help make the email stand out, so you'd be more likely
>> to get a review from me (I missed this thread going by initially).
>>
>>
>
> Oh, ok I will keep that in mind next time.
>
>>  - Emily
>
> Thank you,

Thanks, both.