Re: [PATCH v5 0/5] Convert some stash functionality to a builtin

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 6, 2018, 16:15 UTC
Message-ID: <nycvar.QRO.7.76.6.1804061813240.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <20180405022810.15796-1-joel@teichroeb.net>

Hi Joel,

On Wed, 4 Apr 2018, Joel Teichroeb wrote:

Show 20 quoted lines

> I've been working on converting all of git stash to be a
> builtin, however it's hard to get it all working at once with
> limited time, so I've moved around half of it to a new
> stash--helper builtin and called these functions from the shell
> script. Once this is stabalized, it should be easier to convert
> the rest of the commands one at a time without breaking
> anything.
> 
> I've sent most of this code before, but that was targetting a
> full replacement of stash. The code is overall the same, but
> with some code review changes and updates for internal api
> changes.
> 
> Since there seems to be interest from GSOC students who want to
> work on converting builtins, I figured I should finish what I
> have that works now so they could build on top of it.
> 
> The code is based on next as write_cache_as_tree was changed to
> take an object ID. It can easily be rebase on master by changing
> the two calls to write_cache_as_tree to use tha hash.

Very nice, thank you!

I reviewed all five patches, and offered a couple of suggestion.

As is tradition on this list, I also offered way more comments that do not require any action from you, please use discretion when reading those: I definitely do not expect you to do anything about those "for the future" comments.

This patch series is in a very good shape, methinks, and should not need a whole lot more work to enter `next`. From that point on, a GSoC student working on the `stash` project (if we get any) could take over.

Thank you so much! Dscho