Re: [PATCH] [GSOC] Update MyFirstContribution.adoc to current codebase
- From
D. Ben Knoble <ben.knoble@gmail.com>
- Date
- Mar 13, 2025, 15:46 UTC
- Message-ID
- <CALnO6CCvo+EiabE3LG2pmPZhM=M9_d80Mp_MyozXmHjE9srAWg@mail.gmail.com>
- In-Reply-To
- <20250312081534.75536-1-jayatheerthkulkarni2005@gmail.com>
On Wed, Mar 12, 2025 at 4:15 AM K Jayatheerth <jayatheerthkulkarni2005@gmail.com> wrote:
Show 18 quoted lines
> > This updates MyFirstContribution.adoc to correct outdated information, > improve clarity for new contributors following the guide. > > Key changes: > - Updated the function signature of `cmd_psuh` to match the current Git > codebase, adding `struct repository *repo` as required. > - Added a note on using the `UNUSED` macro to suppress compiler > warnings for unused function parameters. > - Replaced `git_config(...)` with `repo_config(...)` in documentation, > aligning with modern Git practices. > - Removed mention of the deprecated > `git-mentoring@googlegroups.com`. > - Renamed `Documentation/git-psuh.txt` to > `Documentation/git-psuh.adoc` to follow the > correct documentation format. > - Updated `.txt` references to `.adoc` > wherever applicable for consistency.
I'm in a poor position to judge, but I suspect reviewing this commit would be easier if each bullet was a separate commit. See Documentation/SubmittingPatches [[separate-commits]] (HTML: https://git-scm.com/docs/SubmittingPatches#separate-commits).
> > These changes make it easier for > new contributors to follow the tutorial without > running into compilation errors or outdated references.
Agreed as discussed in the other thread, thanks for doing this!
Show 37 quoted lines
> > Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> > --- > Documentation/MyFirstContribution.adoc | 83 +++++++++++++++++--------- > 1 file changed, 55 insertions(+), 28 deletions(-) > > diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc > index afcf4b46c1..4236c9ae3e 100644 > --- a/Documentation/MyFirstContribution.adoc > +++ b/Documentation/MyFirstContribution.adoc > @@ -13,6 +13,7 @@ the Git tree, sending it for review, and making changes based on comments. > > This tutorial assumes you're already fairly familiar with using Git to manage > source code. The Git workflow steps will largely remain unexplained. > +This tutorial also assumes you know/understand C programming language in a good capacity. > > [[related-reading]] > === Related Reading > @@ -40,13 +41,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 > > @@ -149,8 +143,14 @@ subcommand and contained within `builtin/`. So it makes sense to implement your > command in `builtin/psuh.c`. Create that file, and within it, write the entry > point for your command in a function matching the style and signature: > > +The following line represents the function signature for any builtin/<filename.c> file that we add:
This line doesn't look necessary, or should at least replace the sentence immediately prior (which shows up in the hunk context).
Show 147 quoted lines
> +----
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo)
> ----
> -int cmd_psuh(int argc, const char **argv, const char *prefix)
> +Before proceeding further, we should use the UNUSED macro to suppress warnings about unused parameters in the function.
> +This prevents the compiler from generating warnings when certain parameters are not used within the function body:
> +----
> +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
> @@ -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,7 @@ 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;
> @@ -205,6 +205,9 @@ with the command name, a function pointer to the command implementation, and a
> setup option flag. For now, let's keep mimicking `push`. Find the line where
> `cmd_push` is registered, copy it, and modify it for `cmd_psuh`, placing the new
> line in alphabetical order (immediately before `cmd_pull`).
> +----
> +{ "psuh", cmd_psuh, RUN_SETUP}
> +----
>
> The options are documented in `builtin.h` under "Adding a new built-in." Since
> we hope to print some data about the user's current workspace context later,
> @@ -291,6 +294,8 @@ Modify your `cmd_psuh` implementation to dump the args you're passed, keeping
> existing `printf()` calls in place:
>
> ----
> +int cmd_psuh(int argc, const char **argv, const char *prefix, struct repository *repo UNUSED)
> +{
> int i;
>
> ...
> @@ -304,6 +309,7 @@ existing `printf()` calls in place:
>
> printf(_("Your current working directory:\n<top-level>%s%s\n"),
> prefix ? "/" : "", prefix ? prefix : "");
> + ...
>
> ----
>
> @@ -312,26 +318,47 @@ 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 `#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" // Required for repo_config_get_string_tmp()
>
> -...
> +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);
> +
> + 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 : "");
>
> - 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);
> + 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/git-config.adoc`.
>
> You should see that the name printed matches the one you see when you run:
>
> @@ -379,8 +406,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);
>
> ...
>
> @@ -1089,11 +1116,11 @@ The one generated for `psuh` from the sample implementation looks like this:
>
> ----
> Documentation/git-psuh.adoc | 40 +++++++++++++++++++++
> - Makefile | 1 +
> - builtin.h | 1 +
> - builtin/psuh.c | 73 ++++++++++++++++++++++++++++++++++++++
> - git.c | 1 +
> - t/t9999-psuh-tutorial.sh | 12 +++++++
> + Makefile | 1 +
> + builtin.h | 1 +
> + builtin/psuh.c | 73 ++++++++++++++++++++++++++++++++++++++
> + git.c | 1 +
> + t/t9999-psuh-tutorial.sh | 12 +++++++
> 6 files changed, 128 insertions(+)
> create mode 100644 Documentation/git-psuh.adoc
> create mode 100644 builtin/psuh.c
> --
> 2.48.1
>
>-- D. Ben Knoble