[v5,8/9] dir: update stale description of treat_directory()

Commit Message

Derrick Stolee May 12, 2021, 5:28 p.m. UTC

From: Derrick Stolee <[e-mail]>

The documentation comment for treat_directory() was originally written
in 095952 (Teach directory traversal about subprojects, 2007-04-11)
which was before the 'struct dir_struct' split its bitfield of named
options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct
dir_struct into a single variable, 2009-02-16). When those flags
changed, the comment became stale, since members like
'show_other_directories' transitioned into flags like
DIR_SHOW_OTHER_DIRECTORIES.

Update the comments for treat_directory() to use these flag names rather
than the old member names.

Signed-off-by: Derrick Stolee <[e-mail]>
Reviewed-by: Elijah Newren <[e-mail]>
---
 dir.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Derrick Stolee May 17, 2021, 5:20 p.m. UTC | #1

On 5/12/2021 1:28 PM, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[e-mail]>
> 
> The documentation comment for treat_directory() was originally written
> in 095952 (Teach directory traversal about subprojects, 2007-04-11)
> which was before the 'struct dir_struct' split its bitfield of named
> options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct
> dir_struct into a single variable, 2009-02-16). When those flags
> changed, the comment became stale, since members like
> 'show_other_directories' transitioned into flags like
> DIR_SHOW_OTHER_DIRECTORIES.
> 
> Update the comments for treat_directory() to use these flag names rather
> than the old member names.
> 
> Signed-off-by: Derrick Stolee <[e-mail]>
> Reviewed-by: Elijah Newren <[e-mail]>

I think you want the "Reviewed-by" before the "Signed-off-by",
followed by your own sign-off.

Thanks,
-Stolee

Junio C Hamano May 17, 2021, 7:44 p.m. UTC | #2

Derrick Stolee <[e-mail]> writes:

> On 5/12/2021 1:28 PM, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <[e-mail]>
>> 
>> The documentation comment for treat_directory() was originally written
>> in 095952 (Teach directory traversal about subprojects, 2007-04-11)
>> which was before the 'struct dir_struct' split its bitfield of named
>> options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct
>> dir_struct into a single variable, 2009-02-16). When those flags
>> changed, the comment became stale, since members like
>> 'show_other_directories' transitioned into flags like
>> DIR_SHOW_OTHER_DIRECTORIES.
>> 
>> Update the comments for treat_directory() to use these flag names rather
>> than the old member names.
>> 
>> Signed-off-by: Derrick Stolee <[e-mail]>
>> Reviewed-by: Elijah Newren <[e-mail]>
>
> I think you want the "Reviewed-by" before the "Signed-off-by",
> followed by your own sign-off.

Grabbing somebody else's signed-off patch, and forwarding it (with
or without tweaks and enhancements) with your own sign-off would be
a sufficient sign that you've inspected the patch deeply enough to
be confident that it is worth forwarding.  So I think you can even
lose the reviewed-by.

But as long as you are relaying somebody else's patch, DCO asks you
to sign it off yourself.

Thanks.

Elijah Newren May 18, 2021, 3:32 a.m. UTC | #3

On Mon, May 17, 2021 at 12:44 PM Junio C Hamano <[e-mail]> wrote:
>
> Derrick Stolee <[e-mail]> writes:
>
> > On 5/12/2021 1:28 PM, Derrick Stolee via GitGitGadget wrote:
> >> From: Derrick Stolee <[e-mail]>
> >>
> >> The documentation comment for treat_directory() was originally written
> >> in 095952 (Teach directory traversal about subprojects, 2007-04-11)
> >> which was before the 'struct dir_struct' split its bitfield of named
> >> options into a 'flags' enum in 7c4c97c0 (Turn the flags in struct
> >> dir_struct into a single variable, 2009-02-16). When those flags
> >> changed, the comment became stale, since members like
> >> 'show_other_directories' transitioned into flags like
> >> DIR_SHOW_OTHER_DIRECTORIES.
> >>
> >> Update the comments for treat_directory() to use these flag names rather
> >> than the old member names.
> >>
> >> Signed-off-by: Derrick Stolee <[e-mail]>
> >> Reviewed-by: Elijah Newren <[e-mail]>
> >
> > I think you want the "Reviewed-by" before the "Signed-off-by",
> > followed by your own sign-off.
>
> Grabbing somebody else's signed-off patch, and forwarding it (with
> or without tweaks and enhancements) with your own sign-off would be
> a sufficient sign that you've inspected the patch deeply enough to
> be confident that it is worth forwarding.  So I think you can even
> lose the reviewed-by.
>
> But as long as you are relaying somebody else's patch, DCO asks you
> to sign it off yourself.
>
> Thanks.

I was going to go fix this up, but it looks like en/dir-traversal has
already merged down to next.

We could revert the last two patches of the series out of next
(allowing the first seven with the important fixes to merge down) and
then I could resubmit just the last two patches.  Or we could just let
them all merge down as-is.  Preferences?

Junio C Hamano May 19, 2021, 1:44 a.m. UTC | #4

Elijah Newren <[e-mail]> writes:

> We could revert the last two patches of the series out of next
> (allowing the first seven with the important fixes to merge down) and
> then I could resubmit just the last two patches.  Or we could just let
> them all merge down as-is.  Preferences?

The former, thanks.

Patch

diff --git a/dir.c b/dir.c
index c0233bbba36c..4794c822b47f 100644
--- a/dir.c
+++ b/dir.c
@@ -1749,13 +1749,13 @@  static enum exist_status directory_exists_in_index(struct index_state *istate,
  * Case 3: if we didn't have it in the index previously, we
  * have a few sub-cases:
  *
- *  (a) if "show_other_directories" is true, we show it as
- *      just a directory, unless "hide_empty_directories" is
+ *  (a) if DIR_SHOW_OTHER_DIRECTORIES flag is set, we show it as
+ *      just a directory, unless DIR_HIDE_EMPTY_DIRECTORIES is
  *      also true, in which case we need to check if it contains any
  *      untracked and / or ignored files.
- *  (b) if it looks like a git directory, and we don't have
- *      'no_gitlinks' set we treat it as a gitlink, and show it
- *      as a directory.
+ *  (b) if it looks like a git directory and we don't have the
+ *      DIR_NO_GITLINKS flag, then we treat it as a gitlink, and
+ *      show it as a directory.
  *  (c) otherwise, we recurse into it.
  */
 static enum path_treatment treat_directory(struct dir_struct *dir,
@@ -1843,7 +1843,6 @@  static enum path_treatment treat_directory(struct dir_struct *dir,
 		return path_recurse;
 	}
 
-	/* This is the "show_other_directories" case */
 	assert(dir->flags & DIR_SHOW_OTHER_DIRECTORIES);
 
 	/*
@@ -1858,7 +1857,7 @@  static enum path_treatment treat_directory(struct dir_struct *dir,
 	/* Special cases for where this directory is excluded/ignored */
 	if (excluded) {
 		/*
-		 * In the show_other_directories case, if we're not
+		 * If DIR_SHOW_OTHER_DIRECTORIES is set and we're not
 		 * hiding empty directories, there is no need to
 		 * recurse into an ignored directory.
 		 */