Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Calvin Wan <calvinwan@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/6] strbuf cleanups
Date: Tue, 2 May 2023 19:37:00 -0700	[thread overview]
Message-ID: <CABPp-BFagTvBTe0XbEOM_hO-vDFwVvSSmGOii1q2=7amqerwtQ@mail.gmail.com> (raw)
In-Reply-To: <20230502211454.1673000-1-calvinwan@google.com>

On Tue, May 2, 2023 at 2:15 PM Calvin Wan <calvinwan@google.com> wrote:
>
> Strbuf is a basic structure that should function as a low-level library
> due to how generic it is. Over time certain functions inside of
> strbuf.[ch] have been added with dependencies to higher level objects
> and functions. This series cleans up some of those higher level
> dependencies by moving the offending functions to the files they
> interact with. With the goal of eventually being able to stand up strbuf
> as a libary, this series also removes the use of environment variables
> from strbuf.
>
> Calvin Wan (6):
>   abspath: move related functions to abspath
>   credential-store: move related functions to credential-store file
>   object-name: move related functions to object-name
>   path: move related function to path
>   strbuf: clarify dependency
>   strbuf: remove environment variables
>
>  abspath.c                  |  36 +++++++++++++
>  abspath.h                  |  21 ++++++++
>  add-patch.c                |  12 +++--
>  builtin/am.c               |   2 +-
>  builtin/branch.c           |   4 +-
>  builtin/commit.c           |   2 +-
>  builtin/credential-store.c |  19 +++++++
>  builtin/merge.c            |  10 ++--
>  builtin/notes.c            |  16 +++---
>  builtin/rebase.c           |   2 +-
>  builtin/stripspace.c       |   6 ++-
>  builtin/tag.c              |   9 ++--
>  fmt-merge-msg.c            |   9 ++--
>  gpg-interface.c            |   5 +-
>  hook.c                     |   1 +
>  object-name.c              |  15 ++++++
>  object-name.h              |   9 ++++
>  path.c                     |  20 +++++++
>  path.h                     |   5 ++
>  pretty.c                   |   1 +
>  rebase-interactive.c       |  15 +++---
>  sequencer.c                |  24 +++++----
>  strbuf.c                   | 106 +++----------------------------------
>  strbuf.h                   |  44 ++-------------
>  tempfile.c                 |   1 +
>  wt-status.c                |   6 +--
>  26 files changed, 213 insertions(+), 187 deletions(-)
>
> --
> 2.40.1.495.gc816e09b53d-goog

The series looks pretty good to me.  I left a couple small comments on
5/6 and 6/6.  One other high-level note:

As Ævar noted over at
https://lore.kernel.org/git/230501.86a5yohsme.gmgdl@evledraar.gmail.com/,
strbuf_add_separated_string_list() is currently only used by
merge-ort.c and merge-recursive.c (both of which include
merge-recursive.h).  It may make sense to move that function, and that
would permit you to drop the forward declaration of 'struct
string_list;' from strbuf.c as well.

  parent reply	other threads:[~2023-05-03  2:37 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-02 21:14 [PATCH 0/6] strbuf cleanups Calvin Wan
2023-05-02 21:14 ` [PATCH 1/6] abspath: move related functions to abspath Calvin Wan
2023-05-02 21:42   ` Junio C Hamano
2023-05-02 21:14 ` [PATCH 2/6] credential-store: move related functions to credential-store file Calvin Wan
2023-05-02 21:52   ` Junio C Hamano
2023-05-03 16:28   ` Jeff King
2023-05-03 16:34     ` Jeff King
2023-05-03 18:38       ` Calvin Wan
2023-05-02 21:14 ` [PATCH 3/6] object-name: move related functions to object-name Calvin Wan
2023-05-02 21:14 ` [PATCH 4/6] path: move related function to path Calvin Wan
2023-05-02 21:14 ` [PATCH 5/6] strbuf: clarify dependency Calvin Wan
2023-05-03  1:56   ` Elijah Newren
2023-05-02 21:14 ` [PATCH 6/6] strbuf: remove environment variables Calvin Wan
2023-05-03  2:15   ` Elijah Newren
2023-05-02 22:20 ` [PATCH 0/6] strbuf cleanups Junio C Hamano
2023-05-02 22:31   ` Junio C Hamano
2023-05-02 23:51     ` Felipe Contreras
2023-05-02 22:36   ` Calvin Wan
2023-05-03  2:37 ` Elijah Newren [this message]
2023-05-03 18:00   ` Calvin Wan
2023-05-07  0:14     ` Elijah Newren
2023-05-07 13:14       ` Jeff King
2023-05-03 18:48 ` [PATCH v2 0/7] " Calvin Wan
2023-05-03 18:50   ` [PATCH v2 1/7] strbuf: clarify API boundary Calvin Wan
2023-05-03 18:50   ` [PATCH v2 2/7] abspath: move related functions to abspath Calvin Wan
2023-05-03 18:50   ` [PATCH v2 3/7] credential-store: move related functions to credential-store file Calvin Wan
2023-05-03 18:50   ` [PATCH v2 4/7] object-name: move related functions to object-name Calvin Wan
2023-05-03 18:50   ` [PATCH v2 5/7] path: move related function to path Calvin Wan
2023-05-03 18:50   ` [PATCH v2 6/7] strbuf: clarify dependency Calvin Wan
2023-05-03 19:26     ` Junio C Hamano
2023-05-03 18:50   ` [PATCH v2 7/7] strbuf: remove environment variables Calvin Wan
2023-05-03 19:24     ` Junio C Hamano
2023-05-03 19:41       ` Calvin Wan
2023-05-03 19:45         ` Junio C Hamano
2023-05-03 19:42     ` [PATCH v3 7/7] strbuf: remove environment variable Calvin Wan
2023-05-05 22:33   ` [PATCH v2 0/7] strbuf cleanups Junio C Hamano
2023-05-08 16:38     ` Calvin Wan
2023-05-07  0:40   ` Elijah Newren
2023-05-07 21:47     ` Felipe Contreras
2023-05-08 16:57   ` [PATCH v4 " Calvin Wan
2023-05-08 16:59     ` [PATCH v4 1/7] strbuf: clarify API boundary Calvin Wan
2023-05-08 17:22       ` Eric Sunshine
2023-05-10 22:51         ` Junio C Hamano
2023-05-08 16:59     ` [PATCH v4 2/7] abspath: move related functions to abspath Calvin Wan
2023-05-08 16:59     ` [PATCH v4 3/7] credential-store: move related functions to credential-store file Calvin Wan
2023-05-08 16:59     ` [PATCH v4 4/7] object-name: move related functions to object-name Calvin Wan
2023-05-08 16:59     ` [PATCH v4 5/7] path: move related function to path Calvin Wan
2023-05-08 16:59     ` [PATCH v4 6/7] strbuf: clarify dependency Calvin Wan
2023-05-08 16:59     ` [PATCH v4 7/7] strbuf: remove global variable Calvin Wan
2023-05-10  8:12       ` Phillip Wood
2023-05-09  1:57     ` [PATCH v4 0/7] strbuf cleanups Elijah Newren
2023-05-09  2:13       ` Felipe Contreras
2023-05-11 19:44     ` [PATCH v5 " Calvin Wan
2023-05-11 19:48       ` [PATCH v5 1/7] strbuf: clarify API boundary Calvin Wan
2023-05-11 19:57         ` Eric Sunshine
2023-05-11 20:03           ` Calvin Wan
2023-05-11 19:48       ` [PATCH v5 2/7] abspath: move related functions to abspath Calvin Wan
2023-05-11 19:48       ` [PATCH v5 3/7] credential-store: move related functions to credential-store file Calvin Wan
2023-05-11 19:48       ` [PATCH v5 4/7] object-name: move related functions to object-name Calvin Wan
2023-05-11 19:48       ` [PATCH v5 5/7] path: move related function to path Calvin Wan
2023-05-11 19:48       ` [PATCH v5 6/7] strbuf: clarify dependency Calvin Wan
2023-05-11 19:48       ` [PATCH v5 7/7] strbuf: remove global variable Calvin Wan
2023-05-11 20:24         ` Eric Sunshine
2023-05-11 21:42         ` Junio C Hamano
2023-05-12 14:54           ` Phillip Wood
2023-05-12 14:53         ` Phillip Wood
2023-05-12 19:31           ` Junio C Hamano
2023-05-12 17:14       ` [PATCH v6 0/7] strbuf cleanups Calvin Wan
2023-05-12 17:15         ` [PATCH v6 1/7] strbuf: clarify API boundary Calvin Wan
2023-05-12 17:15         ` [PATCH v6 2/7] abspath: move related functions to abspath Calvin Wan
2023-05-12 17:15         ` [PATCH v6 3/7] credential-store: move related functions to credential-store file Calvin Wan
2023-05-12 17:15         ` [PATCH v6 4/7] object-name: move related functions to object-name Calvin Wan
2023-05-12 17:15         ` [PATCH v6 5/7] path: move related function to path Calvin Wan
2023-05-12 17:15         ` [PATCH v6 6/7] strbuf: clarify dependency Calvin Wan
2023-05-12 17:15         ` [PATCH v6 7/7] strbuf: remove global variable Calvin Wan
2023-05-12 20:24         ` [PATCH v6 0/7] strbuf cleanups Junio C Hamano
2023-05-13  5:54           ` Eric Sunshine
2023-06-06 19:47         ` [PATCH v7 " Calvin Wan
2023-06-06 19:48           ` [PATCH v7 1/7] strbuf: clarify API boundary Calvin Wan
2023-06-06 19:48           ` [PATCH v7 2/7] strbuf: clarify dependency Calvin Wan
2023-06-06 19:48           ` [PATCH v7 3/7] abspath: move related functions to abspath Calvin Wan
2023-06-06 19:48           ` [PATCH v7 4/7] credential-store: move related functions to credential-store file Calvin Wan
2023-06-06 19:48           ` [PATCH v7 5/7] object-name: move related functions to object-name Calvin Wan
2023-06-06 19:48           ` [PATCH v7 6/7] path: move related function to path Calvin Wan
2023-06-06 19:48           ` [PATCH v7 7/7] strbuf: remove global variable Calvin Wan

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

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

  git send-email \
    --in-reply-to='CABPp-BFagTvBTe0XbEOM_hO-vDFwVvSSmGOii1q2=7amqerwtQ@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    /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.
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).