git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 3/4] document add_[file_]to_index
Date: Wed, 18 Jan 2017 13:22:42 -0800	[thread overview]
Message-ID: <xmqqbmv43vx9.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170117233503.27137-4-sbeller@google.com> (Stefan Beller's message of "Tue, 17 Jan 2017 15:35:02 -0800")

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  cache.h | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 26632065a5..acc639d6e0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -605,13 +605,20 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>  
>  extern void remove_marked_cache_entries(struct index_state *istate);
>  extern int remove_file_from_index(struct index_state *, const char *path);
> -#define ADD_CACHE_VERBOSE 1
> -#define ADD_CACHE_PRETEND 2
> -#define ADD_CACHE_IGNORE_ERRORS	4
> -#define ADD_CACHE_IGNORE_REMOVAL 8
> -#define ADD_CACHE_INTENT 16
> +
> +#define ADD_CACHE_VERBOSE 1		/* verbose */
> +#define ADD_CACHE_PRETEND 2 		/* dry run */
> +#define ADD_CACHE_IGNORE_ERRORS 4	/* ignore errors */
> +#define ADD_CACHE_IGNORE_REMOVAL 8	/* do not remove files from index */
> +#define ADD_CACHE_INTENT 16		/* intend to add later; stage empty file */

These repeat pretty much the same thing, which is an indication that
the macro names are chosen well not to require extraneous comments
like these, no?

> +/*
> + * Adds the given path the index, respecting the repsitory configuration, e.g.
> + * in case insensitive file systems, the path is normalized.
> + */
>  extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);

s/repsitory/repository/;

> +/* stat the file then call add_to_index */
>  extern int add_file_to_index(struct index_state *, const char *path, int flags);
> +

As you do not say "use the provided stat info to mark the cache
entry up-to-date" in the add_to_index(), I am not sure if mentioning
"stat the file then" has much value.  Besides, you are supposed to
lstat(2) the file, not "stat", no?

I'd cover these two under the same heading and comment if I were
doing this.

	These two are used to add the contents of the file at path
	to the index, marking the working tree up-to-date by storing
	the cached stat info in the resulting cache entry.  A caller
	that has already run lstat(2) on the path can call
	add_to_index(), and all others can call add_file_to_index();
	the latter will do necessary lstat(2) internally before
	calling the former.

or something along that line.

>  extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
>  extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
>  extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);

  reply	other threads:[~2017-01-18 21:23 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-17 23:34 [PATCH 0/4] start documenting core functions Stefan Beller
2017-01-17 23:35 ` [PATCH 1/4] document index_name_pos Stefan Beller
2017-01-18 21:11   ` Junio C Hamano
2017-01-18 22:02     ` Stefan Beller
2017-01-17 23:35 ` [PATCH 2/4] remove_index_entry_at: move documentation to cache.h Stefan Beller
2017-01-18 21:16   ` Junio C Hamano
2017-01-18 22:06     ` Stefan Beller
2017-01-17 23:35 ` [PATCH 3/4] document add_[file_]to_index Stefan Beller
2017-01-18 21:22   ` Junio C Hamano [this message]
2017-01-18 22:09     ` Stefan Beller
2017-01-17 23:35 ` [PATCH 4/4] documentation: retire unfinished documentation Stefan Beller
2017-01-18 21:23   ` Junio C Hamano
2017-01-18 23:21 ` [PATCHv2 0/4] start documenting core functions Stefan Beller
2017-01-18 23:21   ` [PATCHv2 1/4] cache.h: document index_name_pos Stefan Beller
2017-01-19  3:18     ` [PATCHv3 0/4] start documenting core functions Stefan Beller
2017-01-19  3:18       ` [PATCHv3 1/4] cache.h: document index_name_pos Stefan Beller
2017-01-19 20:17         ` Junio C Hamano
2017-01-19  3:18       ` [PATCHv3 2/4] cache.h: document remove_index_entry_at Stefan Beller
2017-01-19  3:18       ` [PATCHv3 3/4] cache.h: document add_[file_]to_index Stefan Beller
2017-01-19  3:18       ` [PATCHv3 4/4] documentation: retire unfinished documentation Stefan Beller
2017-01-18 23:21   ` [PATCHv2 2/4] cache.h: document remove_index_entry_at Stefan Beller
2017-01-18 23:21   ` [PATCHv2 3/4] cache.h: document add_[file_]to_index Stefan Beller
2017-01-19  0:00     ` Brandon Williams
2017-01-19  0:08       ` Stefan Beller
2017-01-18 23:21   ` [PATCHv2 4/4] documentation: retire unfinished documentation Stefan Beller

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbmv43vx9.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).