git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Duy Nguyen <pclouds@gmail.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH v2 24/24] Rename functions to avoid breaking in-flight topics
Date: Tue, 4 Sep 2018 14:12:02 -0700
Message-ID: <CAGZ79kZhQJ7TbTuuDO_k-p80rGnOTjVcHf0aoq2+=pG4bwetNA@mail.gmail.com> (raw)
In-Reply-To: <xmqqo9dd6kx1.fsf@gitster-ct.c.googlers.com>

On Tue, Sep 4, 2018 at 1:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> > The three functions init_revisions(), diff_setup() and rerere() are
> > prefixed temporarily with repo_ to avoid breaking other topics which
> > add new call sites for these functions. This is a temporary
> > measure. Once everything is merged, it will be reverted and the new
> > call sites fixed.
>
> That's a sensible thing to do, but isn't it too late at 24/24 stage?
> IOW, doesn't in-flight topics break if up to 23/24 of this series is
> merged?

Presumably you merge the tip of this series (which contains 24/24)
with the other in-flight topics, that make new uses of
init_revisions(revs, prefix), which 24/24 allows. Going on either
parent side of such a merge will have working commits, that compile.

So from that POV it doesn't matter when the #define is introduced.

> IOW, the one that teaches "work in this repository" to rerere(int)
> for example should have introduced
>
>         repo_rerere(struct repository *, int);
>         #define rerere(x) repo_rerere(the_repository, x)
>
> in its own step, not this late in the series, no?

That might make for a better presentation to reviewers
now and here, but at some later date, we would want to
'git revert 24/24' after fixing all in-flights of today. And with
that in mind it makes sense to separate out all these changes
into this patch, as that allows us to have the revert rename
back to easy names.

You may be asking this question as it was done differently in
the series sb/object-store-lookup. But there the #define's served
a different purpose. That series used the #define to aid
author&reviewer to know where dependencies to the_repository
are and which function is done already.

This series solely uses the #define after the main series is done
to aid the maintainer to merge it which as you noted was one
of the problems with that other approach. The downside of the
series as it is presented here is that it is very easy to miss some
hidden dependency, that leads to a bug only discovered later
(e.g. some function still uses the_repository, when working on
a submodule for example, but as that function is used in some
corner case our test suite would not find it).

So why don't we try this approach as well and see how
well it goes over time?

Thanks,
Stefan

  reply index

Thread overview: 160+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-26 10:02 [PATCH 00/21] Kill the_index part 4 Nguyễn Thái Ngọc Duy
2018-08-26 10:02 ` [PATCH 01/21] archive.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-08-26 10:02 ` [PATCH 02/21] read-cache.c: remove 'const' from index_has_changes() Nguyễn Thái Ngọc Duy
2018-08-27 18:37   ` Stefan Beller
2018-08-28 19:27     ` Duy Nguyen
2018-08-28 20:09       ` Stefan Beller
2018-08-26 10:02 ` [PATCH 03/21] combine-diff.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-08-26 10:02 ` [PATCH 04/21] diff.c: remove the_index dependency in textconv() functions Nguyễn Thái Ngọc Duy
2018-08-26 10:02 ` [PATCH 05/21] grep.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-08-26 10:02 ` [PATCH 06/21] diff.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 07/21] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 08/21] diff-lib.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 09/21] ll-merge.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 10/21] merge-blobs.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 11/21] merge.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 12/21] patch-ids.c: " Nguyễn Thái Ngọc Duy
2018-08-27 19:12   ` Stefan Beller
2018-09-03 18:03     ` Duy Nguyen
2018-09-04 19:54       ` Stefan Beller
2018-09-09  8:01         ` Duy Nguyen
2018-09-10 18:49           ` Stefan Beller
2018-08-26 10:03 ` [PATCH 13/21] sha1-file.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 14/21] rerere.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 15/21] userdiff.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 16/21] line-range.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 17/21] submodule.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 18/21] tree-diff.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 19/21] ws.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 20/21] wt-status.c: " Nguyễn Thái Ngọc Duy
2018-08-26 10:03 ` [PATCH 21/21] wt-status.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-08-27 17:32 ` [PATCH 00/21] Kill the_index part 4 Stefan Beller
2018-08-28 19:32   ` Duy Nguyen
2018-09-03 18:09 ` [PATCH v2 00/24] " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 01/24] archive.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 02/24] read-cache.c: remove 'const' from index_has_changes() Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 03/24] diff.c: reduce implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 04/24] combine-diff.c: remove " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 05/24] blame.c: rename "repo" argument to "r" Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 06/24] diff.c: remove the_index dependency in textconv() functions Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 07/24] grep.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 08/24] diff.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 09/24] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 10/24] diff-lib.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 11/24] ll-merge.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 12/24] merge-blobs.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 13/24] merge.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 14/24] patch-ids.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 15/24] sha1-file.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 16/24] rerere.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 17/24] userdiff.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 18/24] line-range.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 19/24] submodule.c: " Nguyễn Thái Ngọc Duy
2018-10-19 16:20     ` Jeff King
2018-10-19 16:33       ` Duy Nguyen
2018-10-19 16:42         ` Jeff King
2018-10-19 16:53           ` Duy Nguyen
2018-10-19 16:55             ` Jeff King
2018-10-19 17:34               ` [PATCH] submodule.c: remove some of the_repository references Nguyễn Thái Ngọc Duy
2018-10-19 17:46                 ` Stefan Beller
2018-10-19 16:56           ` [PATCH v2 19/24] submodule.c: remove implicit dependency on the_index Duy Nguyen
2018-10-19 16:57           ` Stefan Beller
2018-09-03 18:09   ` [PATCH v2 20/24] tree-diff.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 21/24] ws.c: " Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 22/24] revision.c: " Nguyễn Thái Ngọc Duy
2018-09-04 20:05     ` Stefan Beller
2018-09-04 20:52       ` Junio C Hamano
2018-09-03 18:09   ` [PATCH v2 23/24] revision.c: reduce implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-03 18:09   ` [PATCH v2 24/24] Rename functions to avoid breaking in-flight topics Nguyễn Thái Ngọc Duy
2018-09-04 20:15     ` Stefan Beller
2018-09-04 20:56     ` Junio C Hamano
2018-09-04 21:12       ` Stefan Beller [this message]
2018-09-05 21:04         ` Junio C Hamano
2018-09-09  8:05           ` Duy Nguyen
2018-09-11 16:58             ` Junio C Hamano
2018-09-09  8:53   ` [PATCH v3 00/23] Kill the_index part 4 Nguyễn Thái Ngọc Duy
2018-09-09  8:53     ` [PATCH v3 01/23] archive.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-09  8:53     ` [PATCH v3 02/23] read-cache.c: remove 'const' from index_has_changes() Nguyễn Thái Ngọc Duy
2018-09-09  8:53     ` [PATCH v3 03/23] diff.c: reduce implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-09  8:53     ` [PATCH v3 04/23] combine-diff.c: remove " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 05/23] blame.c: rename "repo" argument to "r" Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 06/23] diff.c: remove the_index dependency in textconv() functions Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 07/23] grep.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 08/23] diff.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 09/23] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 10/23] diff-lib.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 11/23] ll-merge.c: " Nguyễn Thái Ngọc Duy
2018-09-11 16:56       ` Junio C Hamano
2018-09-09  8:54     ` [PATCH v3 12/23] merge-blobs.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 13/23] merge.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 14/23] patch-ids.c: " Nguyễn Thái Ngọc Duy
2018-09-10 18:49       ` Stefan Beller
2018-09-10 19:28         ` Junio C Hamano
2018-09-11 16:05         ` Duy Nguyen
2018-09-09  8:54     ` [PATCH v3 15/23] sha1-file.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 16/23] rerere.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 17/23] userdiff.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 18/23] line-range.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 19/23] submodule.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 20/23] tree-diff.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 21/23] ws.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 22/23] revision.c: " Nguyễn Thái Ngọc Duy
2018-09-09  8:54     ` [PATCH v3 23/23] revision.c: reduce implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-10 18:35     ` [PATCH v3 00/23] Kill the_index part 4 Stefan Beller
2018-09-15 16:17     ` [PATCH v4 " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 01/23] archive.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 02/23] read-cache.c: remove 'const' from index_has_changes() Nguyễn Thái Ngọc Duy
2018-09-17 16:25         ` Junio C Hamano
2018-09-17 16:53           ` Duy Nguyen
2018-09-17 19:13             ` Junio C Hamano
2018-09-15 16:17       ` [PATCH v4 03/23] diff.c: reduce implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 04/23] combine-diff.c: remove " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 05/23] blame.c: rename "repo" argument to "r" Nguyễn Thái Ngọc Duy
2018-09-17 16:32         ` Junio C Hamano
2018-09-15 16:17       ` [PATCH v4 06/23] diff.c: remove the_index dependency in textconv() functions Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 07/23] grep.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 08/23] diff.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 09/23] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 10/23] diff-lib.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 11/23] ll-merge.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 12/23] merge-blobs.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 13/23] merge.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 14/23] patch-ids.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 15/23] sha1-file.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 16/23] rerere.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 17/23] userdiff.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 18/23] line-range.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 19/23] submodule.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 20/23] tree-diff.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 21/23] ws.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 22/23] revision.c: " Nguyễn Thái Ngọc Duy
2018-09-15 16:17       ` [PATCH v4 23/23] revision.c: reduce implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-21 15:57       ` [PATCH v5 00/23] Kill the_index part 4 Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 01/23] archive.c: remove implicit dependency the_repository Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 02/23] read-cache.c: remove 'const' from index_has_changes() Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 03/23] diff.c: reduce implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 04/23] combine-diff.c: remove " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 05/23] blame.c: rename "repo" argument to "r" Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 06/23] diff.c: remove the_index dependency in textconv() functions Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 07/23] grep.c: remove implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 08/23] diff.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 09/23] read-cache.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 10/23] diff-lib.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 11/23] ll-merge.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 12/23] merge-blobs.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 13/23] merge.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 14/23] patch-ids.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 15/23] sha1-file.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 16/23] rerere.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 17/23] userdiff.c: " Nguyễn Thái Ngọc Duy
2018-09-21 16:46           ` Junio C Hamano
2018-10-10 14:51           ` Jeff King
2018-10-10 22:14             ` Junio C Hamano
2018-10-11  0:10               ` Jeff King
2018-10-14 12:33             ` Duy Nguyen
2018-09-21 15:57         ` [PATCH v5 18/23] line-range.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 19/23] submodule.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 20/23] tree-diff.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 21/23] ws.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 22/23] revision.c: " Nguyễn Thái Ngọc Duy
2018-09-21 15:57         ` [PATCH v5 23/23] revision.c: reduce implicit dependency the_repository Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publically 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='CAGZ79kZhQJ7TbTuuDO_k-p80rGnOTjVcHf0aoq2+=pG4bwetNA@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox