git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: David Turner <David.Turner@twosigma.com>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>,
	"bmwill@google.com" <bmwill@google.com>,
	"gitster@pobox.com" <gitster@pobox.com>,
	"jrnieder@gmail.com" <jrnieder@gmail.com>,
	"mogulguy10@gmail.com" <mogulguy10@gmail.com>
Subject: Re: [PATCH 07/16] update submodules: introduce submodule_is_interesting
Date: Thu, 17 Nov 2016 12:03:41 -0800	[thread overview]
Message-ID: <CAGZ79kbs1ymw7mE6ctGt2_wgHoFSYMbuhcFK_3vGR9tYZzJxOg@mail.gmail.com> (raw)
In-Reply-To: <e748abfad8b04a8eaaa10797d9324891@exmbdft7.ad.twosigma.com>

On Tue, Nov 15, 2016 at 4:14 PM, David Turner <David.Turner@twosigma.com> wrote:

>> +int submodule_is_interesting(const char *path, const unsigned char
>> +*sha1) {
>
> This is apparently only ever (in this series) called with null_sha1.  So either this arg is unnecessary, or there are bugs elsewhere in the code.

I was torn when writing the series, as I initially had submodule_is_interesting
with no sha1 argument and it turned out to be buggy in my first
initial implementation,
which lead me to thinking the sha1 actually matters.

The line of thinking was similar to loading the submodules from the
submodule-config cache as that also has different values for different sha1s,
e.g. a submodule is only interesting if submodule.<name>.update != none,
which can have changed with different sha1s.

I refactored the series since then to call the _is_initeresting method
at different times
(before and after the actual checkout), such that we implicitly have
the correct sha1
while calling it.

So I would argue the sha1 argument is not needed. I'll remove it.

  reply	other threads:[~2016-11-17 20:03 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-15 23:06 [RFC PATCH 00/16] Checkout aware of Submodules! Stefan Beller
2016-11-15 23:06 ` [PATCH 01/16] submodule.h: add extern keyword to functions, break line before 80 Stefan Beller
2016-11-16 19:08   ` Junio C Hamano
2016-11-17 18:29     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 02/16] submodule: modernize ok_to_remove_submodule to use argv_array Stefan Beller
2016-11-15 23:11   ` David Turner
2016-11-16 19:03     ` Junio C Hamano
2016-11-17 18:36       ` Stefan Beller
2016-11-15 23:06 ` [PATCH 03/16] submodule: use absolute path for computing relative path connecting Stefan Beller
2016-11-15 23:06 ` [PATCH 04/16] update submodules: add is_submodule_populated Stefan Beller
2016-11-15 23:20   ` Brandon Williams
2016-11-15 23:06 ` [PATCH 05/16] update submodules: add submodule config parsing Stefan Beller
2016-11-15 23:06 ` [PATCH 06/16] update submodules: add a config option to determine if submodules are updated Stefan Beller
2016-11-15 23:06 ` [PATCH 07/16] update submodules: introduce submodule_is_interesting Stefan Beller
2016-11-15 23:34   ` Brandon Williams
2016-11-16  0:14   ` David Turner
2016-11-17 20:03     ` Stefan Beller [this message]
     [not found]   ` <20161117105715.GC39230@book.hvoigt.net>
2016-11-17 20:08     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 08/16] update submodules: add depopulate_submodule Stefan Beller
2016-11-15 23:44   ` Brandon Williams
2016-11-17 22:23     ` Stefan Beller
2016-11-17 22:29       ` Brandon Williams
2016-11-17 22:42         ` Stefan Beller
2016-11-18  0:16           ` Stefan Beller
2016-11-18 17:46             ` Brandon Williams
2016-11-18 18:25               ` Stefan Beller
     [not found]   ` <20161117111337.GD39230@book.hvoigt.net>
2016-11-17 22:28     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 09/16] update submodules: add scheduling to update submodules Stefan Beller
2016-11-16  0:02   ` Brandon Williams
2016-11-16  0:07     ` David Turner
2016-11-18  0:28     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 10/16] update submodules: is_submodule_checkout_safe Stefan Beller
2016-11-16  0:06   ` Brandon Williams
2016-11-15 23:06 ` [PATCH 11/16] teach unpack_trees() to remove submodule contents Stefan Beller
2016-11-16  0:14   ` Brandon Williams
     [not found]   ` <20161117133538.GF39230@book.hvoigt.net>
2016-11-18 19:25     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 12/16] entry: write_entry to write populate submodules Stefan Beller
2016-11-15 23:06 ` [PATCH 13/16] submodule: teach unpack_trees() to update submodules Stefan Beller
2016-11-16  0:22   ` David Turner
2016-11-18 23:33     ` Stefan Beller
2016-11-21 18:12       ` David Turner
2016-11-16  0:25   ` Brandon Williams
2016-11-18 23:39     ` Stefan Beller
2016-11-15 23:06 ` [PATCH 14/16] checkout: recurse into submodules if asked to Stefan Beller
2016-11-16  0:33   ` Brandon Williams
2016-11-16 17:03   ` David Turner
2016-11-16 17:05   ` David Turner
2016-11-15 23:06 ` [PATCH 15/16] completion: add '--recurse-submodules' to checkout Stefan Beller
2016-11-15 23:06 ` [PATCH 16/16] checkout: add config option to recurse into submodules by default Stefan Beller
2016-12-03  6:13 ` [RFC PATCH 00/16] Checkout aware of Submodules! Xiaodong Qi

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=CAGZ79kbs1ymw7mE6ctGt2_wgHoFSYMbuhcFK_3vGR9tYZzJxOg@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=David.Turner@twosigma.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mogulguy10@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
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).