From: Junio C Hamano <gitster@pobox•com>
To: Lars Schneider <larsxschneider@gmail•com>
Cc: git@vger•kernel.org, peff@peff•net, tboegi@web•de, e@80x24•org,
ttaylorr@github•com, peartben@gmail•com
Subject: Re: [PATCH v7 6/6] convert: add "status=delayed" to filter process protocol
Date: Tue, 27 Jun 2017 14:30:39 -0700 [thread overview]
Message-ID: <xmqqlgodyv7k.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <77DBCB49-2A44-42BB-9060-23FA79C1CF57@gmail.com> (Lars Schneider's message of "Tue, 27 Jun 2017 22:34:50 +0200")
Lars Schneider <larsxschneider@gmail•com> writes:
> I treated that as low-prio as I got the impression that you are generally OK with
> the current implementation. I promise to send a patch with this change.
> Either as part of this series or as a follow up patch.
Sure. I was just being curious what happened; I agree that it is a
good idea to treat this as a lower priority clean-up item and leave
it to later.
> Yes. I guess that was a premature optimization on my end.
> This is how "write_entry()" in entry.c would change:
>
> - dco->is_delayed = 0;
> ret = async_convert_to_working_tree(
> ce->name, new, size, &buf, dco);
> - if (ret && dco->is_delayed) {
> + if (ret && string_list_has_string(&dco->paths, ce->name)) {
> free(new);
> goto finish;
> }
>
> OK?
Actually I should be asking the "OK?" question---the above was what
I had in mind when I commented, but you are a lot more familiar with
the way how this code is supposed to work, and I didn't know if the
emptyness of the list can be a 100% substitute for that bit. If you
think they are equivalent, that is a great news.
> At this point I want to ensure that checkout_entry() is called
> with the CE_RETRY state. Because checkout_entry() needs to pass
> that flag to the convert machinery.
>
> This assert was useful when checkout_entry() changed dco->state
> between CAN_DELAY and DELAYED. In the current implementation the
> state should not be changed anymore.
>
> Would you prefer if I remove it? (with or without is both fine with me)
If that is the reason behind the assert, I am OK with either (1)
moving it _after_ checkout_entry() returns (to ensure that the
function no longer mucks with the state, or (2) removing it.
Leaving it at the current position does not make much sense to me.
next prev parent reply other threads:[~2017-06-27 21:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-27 12:10 [PATCH v7 0/6] convert: add "status=delayed" to filter process protocol Lars Schneider
2017-06-27 12:10 ` [PATCH v7 1/6] t0021: keep filter log files on comparison Lars Schneider
2017-06-27 12:10 ` [PATCH v7 2/6] t0021: make debug log file name configurable Lars Schneider
2017-06-27 12:10 ` [PATCH v7 3/6] t0021: write "OUT <size>" only on success Lars Schneider
2017-06-27 18:44 ` Junio C Hamano
2017-06-27 20:51 ` Lars Schneider
2017-06-27 21:26 ` Junio C Hamano
2017-06-27 12:10 ` [PATCH v7 4/6] convert: put the flags field before the flag itself for consistent style Lars Schneider
2017-06-27 12:10 ` [PATCH v7 5/6] convert: move multiple file filter error handling to separate function Lars Schneider
2017-06-27 12:10 ` [PATCH v7 6/6] convert: add "status=delayed" to filter process protocol Lars Schneider
2017-06-27 19:00 ` Junio C Hamano
2017-06-27 20:34 ` Lars Schneider
2017-06-27 21:30 ` Junio C Hamano [this message]
2017-06-27 21:58 ` Lars Schneider
2017-06-27 21:49 ` Lars Schneider
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=xmqqlgodyv7k.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=e@80x24$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=larsxschneider@gmail$(echo .)com \
--cc=peartben@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=tboegi@web$(echo .)de \
--cc=ttaylorr@github$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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