[2/2] docs: add headers in MyFirstObjectWalk

Commit Message

John Cai Oct. 27, 2021, 2:05 p.m. UTC

From: John Cai <[e-mail]>

In several places, headers need to be included or else the code won't
compile. Since this is the first object walk, it would be nice to
include them in the tutorial to make it easier to follow.

Signed-off-by: John Cai <[e-mail]>
---
 Documentation/MyFirstObjectWalk.txt | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

Eric Sunshine Oct. 27, 2021, 5:09 p.m. UTC | #1

On Wed, Oct 27, 2021 at 10:05 AM John Cai via GitGitGadget
<[e-mail]> wrote:
> In several places, headers need to be included or else the code won't
> compile. Since this is the first object walk, it would be nice to
> include them in the tutorial to make it easier to follow.
>
> Signed-off-by: John Cai <[e-mail]>
> ---
> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt
> @@ -57,9 +57,14 @@ command). So we will send our debug output to `trace_printf()` instead. When
>  Add usage text and `-h` handling, like all subcommands should consistently do
> -(our test suite will notice and complain if you fail to do so).
> +(our test suite will notice and complain if you fail to do so). We'll need to include
> +the "parse-options.h" header.

Makes sense, but we should probably typeset these consistently with
backticks rather than double quotes...

> @@ -195,9 +200,13 @@ Similarly to the default values, we don't have anything to do here yet
> -Add a new function to `builtin/walken.c`:
> +Add a new function to `builtin/walken.c`. We'll also need to include the "config.h" header:

... just as `builtin/walken.c` is typeset with backticks (as are other
pathnames in this document).

> @@ -229,8 +238,14 @@ typically done by calling `repo_init_revisions()` with the repository you intend
> -Add the `struct rev_info` and the `repo_init_revisions()` call:
> +Add the `struct rev_info` and the `repo_init_revisions()` call. We'll also need to include
> +the "revision.h" header:

And so on through the remainder of the patch.

Thanks.

Junio C Hamano Oct. 27, 2021, 9:17 p.m. UTC | #2

"John Cai via GitGitGadget" <[e-mail]> writes:

>  Add usage text and `-h` handling, like all subcommands should consistently do
> -(our test suite will notice and complain if you fail to do so).
> +(our test suite will notice and complain if you fail to do so). We'll need to include
> +the "parse-options.h" header.

OK, but wrap this overlong line.  All the lines updated by this
patch, except the one in the last hunk, have become overly long.

[jc: cc'ed the primary author of the document to sanity checking]

Thanks.

>  
>  ----
> +#include "parse-options.h"
> +
> +...
> +
>  int cmd_walken(int argc, const char **argv, const char *prefix)
>  {
>  	const char * const walken_usage[] = {
> @@ -195,9 +200,13 @@ Similarly to the default values, we don't have anything to do here yet
>  ourselves; however, we should call `git_default_config()` if we aren't calling
>  any other existing config callbacks.
>  
> -Add a new function to `builtin/walken.c`:
> +Add a new function to `builtin/walken.c`. We'll also need to include the "config.h" header:
>  
>  ----
> +#include "config.h"
> +
> +...
> +
>  static int git_walken_config(const char *var, const char *value, void *cb)
>  {
>  	/*
> @@ -229,8 +238,14 @@ typically done by calling `repo_init_revisions()` with the repository you intend
>  to target, as well as the `prefix` argument of `cmd_walken` and your `rev_info`
>  struct.
>  
> -Add the `struct rev_info` and the `repo_init_revisions()` call:
> +Add the `struct rev_info` and the `repo_init_revisions()` call. We'll also need to include
> +the "revision.h" header:
> +
>  ----
> +#include "revision.h"
> +
> +...
> +
>  int cmd_walken(int argc, const char **argv, const char *prefix)
>  {
>  	/* This can go wherever you like in your declarations.*/
> @@ -624,9 +639,14 @@ static void walken_object_walk(struct rev_info *rev)
>  ----
>  
>  Let's start by calling just the unfiltered walk and reporting our counts.
> -Complete your implementation of `walken_object_walk()`:
> +Complete your implementation of `walken_object_walk()`. We'll also need to
> +include the "list-objects.h" header.
>  
>  ----
> +#include "list-objects.h"
> +
> +...
> +
>  	traverse_commit_list(rev, walken_show_commit, walken_show_object, NULL);
>  
>  	printf("commits %d\nblobs %d\ntags %d\ntrees %d\n", commit_count,

Bagas Sanjaya Oct. 30, 2021, 7:33 a.m. UTC | #3

On 27/10/21 21.05, John Cai via GitGitGadget wrote:
> From: John Cai <[e-mail]>
> 
> In several places, headers need to be included or else the code won't
> compile. Since this is the first object walk, it would be nice to
> include them in the tutorial to make it easier to follow.
> 

Patch title should be "#include missing headers" (dunno?)

Eric Sunshine Oct. 30, 2021, 7:49 a.m. UTC | #4

On Sat, Oct 30, 2021 at 3:33 AM Bagas Sanjaya <[e-mail]> wrote:
> On 27/10/21 21.05, John Cai via GitGitGadget wrote:
> > In several places, headers need to be included or else the code won't
> > compile. Since this is the first object walk, it would be nice to
> > include them in the tutorial to make it easier to follow.
>
> Patch title should be "#include missing headers" (dunno?)

If the series was being rerolled for some other reason, making the
commit subject slightly more precise like that might make sense, but
rerolling only for that reason is probably not worth the extra work
for the submitter, the maintainer, and reviewers.

Patch

diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt
index bf0a7c1f766..ba8cca91b97 100644
--- a/Documentation/MyFirstObjectWalk.txt
+++ b/Documentation/MyFirstObjectWalk.txt
@@ -57,9 +57,14 @@  command). So we will send our debug output to `trace_printf()` instead. When
 running, enable trace output by setting the environment variable `GIT_TRACE`.
 
 Add usage text and `-h` handling, like all subcommands should consistently do
-(our test suite will notice and complain if you fail to do so).
+(our test suite will notice and complain if you fail to do so). We'll need to include
+the "parse-options.h" header.
 
 ----
+#include "parse-options.h"
+
+...
+
 int cmd_walken(int argc, const char **argv, const char *prefix)
 {
 	const char * const walken_usage[] = {
@@ -195,9 +200,13 @@  Similarly to the default values, we don't have anything to do here yet
 ourselves; however, we should call `git_default_config()` if we aren't calling
 any other existing config callbacks.
 
-Add a new function to `builtin/walken.c`:
+Add a new function to `builtin/walken.c`. We'll also need to include the "config.h" header:
 
 ----
+#include "config.h"
+
+...
+
 static int git_walken_config(const char *var, const char *value, void *cb)
 {
 	/*
@@ -229,8 +238,14 @@  typically done by calling `repo_init_revisions()` with the repository you intend
 to target, as well as the `prefix` argument of `cmd_walken` and your `rev_info`
 struct.
 
-Add the `struct rev_info` and the `repo_init_revisions()` call:
+Add the `struct rev_info` and the `repo_init_revisions()` call. We'll also need to include
+the "revision.h" header:
+
 ----
+#include "revision.h"
+
+...
+
 int cmd_walken(int argc, const char **argv, const char *prefix)
 {
 	/* This can go wherever you like in your declarations.*/
@@ -624,9 +639,14 @@  static void walken_object_walk(struct rev_info *rev)
 ----
 
 Let's start by calling just the unfiltered walk and reporting our counts.
-Complete your implementation of `walken_object_walk()`:
+Complete your implementation of `walken_object_walk()`. We'll also need to
+include the "list-objects.h" header.
 
 ----
+#include "list-objects.h"
+
+...
+
 	traverse_commit_list(rev, walken_show_commit, walken_show_object, NULL);
 
 	printf("commits %d\nblobs %d\ntags %d\ntrees %d\n", commit_count,