From: Emily Shaffer <nasamuffin@google.com>
To: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 3/4] Docs: Add cmd_psuh with repo and UNUSED removal
Date: Fri, 16 May 2025 11:12:50 -0700 [thread overview]
Message-ID: <aCeAIqwvEVOdrsMg@google.com> (raw)
In-Reply-To: <20250416061450.25695-4-jayatheerthkulkarni2005@gmail.com>
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
>
>
next prev parent reply other threads:[~2025-05-16 18:12 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-16 6:14 [PATCH 0/4] update MyFirstContribution with current code base K Jayatheerth
2025-04-16 6:14 ` [PATCH 1/4] Remove unused git-mentoring mailing list K Jayatheerth
2025-04-16 6:14 ` [PATCH 2/4] Docs: Correct cmd_psuh and Explain UNUSED macro K Jayatheerth
2025-05-16 18:08 ` Emily Shaffer
2025-04-16 6:14 ` [PATCH 3/4] Docs: Add cmd_psuh with repo and UNUSED removal K Jayatheerth
2025-05-16 18:12 ` Emily Shaffer [this message]
2025-05-16 18:55 ` [PATCH v2 1/3] docs: remove unused mentoring mailing list reference K Jayatheerth
2025-05-16 18:55 ` [PATCH v2 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro K Jayatheerth
2025-05-17 13:39 ` Junio C Hamano
2025-05-17 18:39 ` Junio C Hamano
2025-05-18 7:34 ` [PATCH v3 0/3] Update MyFirstContribution.adoc to follow modern practices K Jayatheerth
2025-05-18 7:34 ` [PATCH v3 1/3] docs: remove unused mentoring mailing list reference K Jayatheerth
2025-05-18 7:34 ` [PATCH v3 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro K Jayatheerth
2025-05-18 7:34 ` [PATCH v3 3/3] docs: replace git_config to repo_config K Jayatheerth
2025-05-18 7:40 ` JAYATHEERTH K
2025-05-16 18:55 ` [PATCH v2 " K Jayatheerth
2025-05-17 13:39 ` Junio C Hamano
2025-05-16 20:57 ` [PATCH v2 1/3] docs: remove unused mentoring mailing list reference Emily Shaffer
2025-05-17 1:19 ` Junio C Hamano
2025-05-17 3:36 ` [PATCH v3 0/3] Update MyFirstContribution.adoc to follow modern practices K Jayatheerth
2025-05-17 3:36 ` [PATCH v3 1/3] docs: remove unused mentoring mailing list reference K Jayatheerth
2025-05-17 3:36 ` [PATCH v3 2/3] docs: clarify cmd_psuh signature and explain UNUSED macro K Jayatheerth
2025-05-17 3:36 ` [PATCH v3 3/3] docs: replace git_config to repo_config K Jayatheerth
2025-04-16 6:14 ` [PATCH 4/4] cmd_psuh: Prefer repo_config for config lookup K Jayatheerth
2025-05-16 18:26 ` Emily Shaffer
2025-05-16 19:06 ` JAYATHEERTH K
2025-04-16 14:16 ` [PATCH 0/4] update MyFirstContribution with current code base Junio C Hamano
2025-04-16 14:38 ` JAYATHEERTH K
2025-05-13 11:17 ` JAYATHEERTH K
2025-05-14 12:48 ` Junio C Hamano
2025-05-14 13:06 ` JAYATHEERTH K
2025-05-15 22:38 ` Emily Shaffer
2025-05-16 8:20 ` JAYATHEERTH K
2025-05-16 16:11 ` Junio C Hamano
2025-05-16 17:18 ` JAYATHEERTH K
2025-05-20 14:15 ` D. Ben Knoble
2025-05-17 18:24 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aCeAIqwvEVOdrsMg@google.com \
--to=nasamuffin@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jayatheerthkulkarni2005@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.