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

[Joel Teichroeb]

Re-sending, but this time in plain text (why doesn't gmail for android
support that...)

On Mon, Apr 30, 2018 at 7:35 AM, Joel Teichroeb  wrote:
> Hi Paul,
>
> That sounds great to me!
>
> Thanks,
> Joel
>
>
> On Sat, Apr 28, 2018, 3:06 PM Paul-Sebastian Ungureanu
>  wrote:
>>
>> Hello Joel,
>>
>> >> 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.
>>
>> First of all, I must thank you for submitting this series of patches. It
>> is a great starting point to convert 'git stash'.
>>
>> I would like to continue your work on "git stash" if that is fine with
>> you. I will continue to build on top of it, starting with applying some
>> patches in order to implement what was already suggested in this thread.
>>   During the summer, I am planning to finish this process and,
>> hopefully, have a 100% C, built-in 'git stash'.
>>
>> Best regards,
>> Paul

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

[Johannes Schindelin]

Hi,

On Sun, 29 Apr 2018, Paul-Sebastian Ungureanu wrote:

> > > 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.
> 
> First of all, I must thank you for submitting this series of patches. It is a
> great starting point to convert 'git stash'.
> 
> I would like to continue your work on "git stash" if that is fine with you.

So you mean you would like to take custody of Joel's patches and address
the reviews so far, then re-submit?

Ciao,
Dscho

P.S.: Yes, I think I know that you want to do that (as we discussed on
IRC), but I just wanted to clarify on the mailing list, to avoid
misunderstandings ;-).

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

[Paul-Sebastian Ungureanu]

Hello Joel,


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.


First of all, I must thank you for submitting this series of patches. It 
is a great starting point to convert 'git stash'.


I would like to continue your work on "git stash" if that is fine with 
you. I will continue to build on top of it, starting with applying some 
patches in order to implement what was already suggested in this thread. 
 During the summer, I am planning to finish this process and, 
hopefully, have a 100% C, built-in 'git stash'.


Best regards,
Paul

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

[Johannes Schindelin]

Hi Joel,


On Wed, 4 Apr 2018, Joel Teichroeb wrote:

> 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

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

[Joel Teichroeb]

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.

Previous threads:
v1: https://public-inbox.org/git/20180325173916.GE10909@hank/T/
v2: https://public-inbox.org/git/20180326011426.19159-1-j...@teichroeb.net/
v3: https://public-inbox.org/git/20180327054432.26419-1-j...@teichroeb.net/
v4: https://public-inbox.org/git/20180328222129.22192-1-j...@teichroeb.net/

Changes from v4:
 - Fixed a typo (Thanks Eric)
 - Redid my test again with input from Junio
 - Cleaned up usages of get_oid (Thanks Junio)
 - Slightly reduced calls to builtin functions (rerere, rev-parse)
 - Added comments clarifying why when forking/cmd_* is used

Joel Teichroeb (5):
  stash: improve option parsing test coverage
  stash: convert apply to builtin
  stash: convert drop and clear to builtin
  stash: convert branch to builtin
  stash: convert pop to builtin

 .gitignore  |   1 +
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/stash--helper.c | 630 
 git-stash.sh| 136 +--
 git.c   |   1 +
 t/t3903-stash.sh|  35 +++
 7 files changed, 677 insertions(+), 128 deletions(-)
 create mode 100644 builtin/stash--helper.c

-- 
2.16.3