git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: nbelakovski@gmail.com
Cc: git@vger.kernel.org, sunshine@sunshineco.com
Subject: Re: [PATCH v3 2/2] worktree: rename is_worktree_locked to worktree_lock_reason
Date: Wed, 31 Oct 2018 11:41:27 +0900	[thread overview]
Message-ID: <xmqqh8h2zvl4.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20181030062409.42169-2-nbelakovski@gmail.com> (nbelakovski's message of "Mon, 29 Oct 2018 23:24:09 -0700")

nbelakovski@gmail.com writes:

> From: Nickolai Belakovski <nbelakovski@gmail.com>
>
> A function prefixed with 'is_' would be expected to return a boolean,
> however this function returns a string.
>
> Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
> ---

Given that there is a clear documentation in worktree.h, and a
pointer that is not NULL is true in "if/while(ptr)", I'd say this
change is a borderline "meh".

I'll queue this on top of 1/2 and try merging to the integration
topics to see if it interacts with any other topics in flight.
Since the patch has already been written, let's not waste the effort
if there is no conflict with anybody else (otherwise I'd discard
this one---a "meh" patch is not worth having to worry about conflict
resolution).

Thanks.

>  builtin/worktree.c | 10 +++++-----
>  worktree.c         |  2 +-
>  worktree.h         |  4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index c4abbde2b..5e8402617 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -245,7 +245,7 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)
>  	if (!wt)
>  		goto done;
>  
> -	locked = !!is_worktree_locked(wt);
> +	locked = !!worktree_lock_reason(wt);
>  	if ((!locked && opts->force) || (locked && opts->force > 1)) {
>  		if (delete_git_dir(wt->id))
>  		    die(_("unable to re-add worktree '%s'"), path);
> @@ -682,7 +682,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix)
>  	if (is_main_worktree(wt))
>  		die(_("The main working tree cannot be locked or unlocked"));
>  
> -	old_reason = is_worktree_locked(wt);
> +	old_reason = worktree_lock_reason(wt);
>  	if (old_reason) {
>  		if (*old_reason)
>  			die(_("'%s' is already locked, reason: %s"),
> @@ -714,7 +714,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
>  		die(_("'%s' is not a working tree"), av[0]);
>  	if (is_main_worktree(wt))
>  		die(_("The main working tree cannot be locked or unlocked"));
> -	if (!is_worktree_locked(wt))
> +	if (!worktree_lock_reason(wt))
>  		die(_("'%s' is not locked"), av[0]);
>  	ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
>  	free_worktrees(worktrees);
> @@ -787,7 +787,7 @@ static int move_worktree(int ac, const char **av, const char *prefix)
>  	validate_no_submodules(wt);
>  
>  	if (force < 2)
> -		reason = is_worktree_locked(wt);
> +		reason = worktree_lock_reason(wt);
>  	if (reason) {
>  		if (*reason)
>  			die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"),
> @@ -900,7 +900,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
>  	if (is_main_worktree(wt))
>  		die(_("'%s' is a main working tree"), av[0]);
>  	if (force < 2)
> -		reason = is_worktree_locked(wt);
> +		reason = worktree_lock_reason(wt);
>  	if (reason) {
>  		if (*reason)
>  			die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),
> diff --git a/worktree.c b/worktree.c
> index b0d0b5426..befdbe7fa 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -235,7 +235,7 @@ int is_main_worktree(const struct worktree *wt)
>  	return !wt->id;
>  }
>  
> -const char *is_worktree_locked(struct worktree *wt)
> +const char *worktree_lock_reason(struct worktree *wt)
>  {
>  	assert(!is_main_worktree(wt));
>  
> diff --git a/worktree.h b/worktree.h
> index 6b12a3cf6..55d449b6a 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -10,7 +10,7 @@ struct worktree {
>  	char *path;
>  	char *id;
>  	char *head_ref;		/* NULL if HEAD is broken or detached */
> -	char *lock_reason;	/* private - use is_worktree_locked */
> +	char *lock_reason;	/* private - use worktree_lock_reason */
>  	struct object_id head_oid;
>  	int is_detached;
>  	int is_bare;
> @@ -60,7 +60,7 @@ extern int is_main_worktree(const struct worktree *wt);
>   * Return the reason string if the given worktree is locked or NULL
>   * otherwise.
>   */
> -extern const char *is_worktree_locked(struct worktree *wt);
> +extern const char *worktree_lock_reason(struct worktree *wt);
>  
>  #define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0)

  reply	other threads:[~2018-10-31  2:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24  6:39 [PATCH] worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree files nbelakovski
2018-10-24  8:11 ` Eric Sunshine
2018-10-25  5:46   ` Nickolai Belakovski
2018-10-25  5:51     ` [PATCH] worktree: refactor lock_reason_valid and lock_reason to be more sensible nbelakovski
2018-10-25  6:56       ` Junio C Hamano
2018-10-28 21:56         ` Nickolai Belakovski
2018-10-29  3:52           ` Junio C Hamano
2018-10-29  5:43             ` Nickolai Belakovski
2018-10-29  6:42               ` Junio C Hamano
     [not found]         ` <CAC05386cSUhBm4TLD5NUeb5Ut9GT5=h-1MvqDnFpuc+UdZFmwg@mail.gmail.com>
2018-10-28 23:02           ` Eric Sunshine
2018-10-29  1:10             ` Nickolai Belakovski
2018-10-29  4:01               ` Eric Sunshine
2018-10-29  5:45                 ` Nickolai Belakovski
2018-10-29  6:21                   ` Eric Sunshine
2018-10-30  6:24       ` [PATCH v3 1/2] worktree: update documentation for lock_reason and lock_reason_valid nbelakovski
2018-10-31  2:28         ` Junio C Hamano
2018-10-30  6:24       ` [PATCH v3 2/2] worktree: rename is_worktree_locked to worktree_lock_reason nbelakovski
2018-10-31  2:41         ` Junio C Hamano [this message]
2018-10-25 19:14     ` [PATCH] worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree files Eric Sunshine

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=xmqqh8h2zvl4.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=nbelakovski@gmail.com \
    --cc=sunshine@sunshineco.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).