Re: [GSoC] Convert “git stash” to builtin proposal

From: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
Date: Mar 24, 2018, 19:31 UTC
Message-ID: <6603149f-776a-fde8-5d11-a7d9d6d37e96@gmail.com>
In-Reply-To: <CAP8UFD3bRaPke8MvubZ3+v6RrY7K7Peip1dpQ2LG9kxKoXcmbw@mail.gmail.com>

On 23.03.2018 19:11, Christian Couder wrote:

Show 5 quoted lines

>> * Ensure that no regression occurred: considering that there are plenty
>> of tests and that I have a good understanding of the function, this
>> should be a trivial task.
> 
> There are a lot of things that the test suite doesn't test.

Hopefully, by first adding new tests, any eventual bug will be spotted.

Show 9 quoted lines

>> * In the end, an analysis based on performance can be made.
>> Benchmarking the script will be done by recording the running time
>> given a large set of diversified tests that cover all the
>> functionalities, before and after conversion. The new commands should
>> run faster just because they were written in C, but I expect to see
>> even more improvements.
> 
> If you want to do benchmarks, you might want to add performance tests
> into t/perf.

That is great. I will surely make use of the existent testing framework to do benchmarks.

Show 10 quoted lines

>> ## Example of conversion (for “git stash list”)
>> ------------------------------------------
>> To test my capabilities and to be sure that I am able to work on a
>> project of this kind, I tried to convert “git stash list” into a built-
>> in. The result can be found below in text-only version or at the Github
>> link.
> 
> I think it would be better if it was sent as a patch (maybe an RFC
> patch) to the mailing list and add the link to the patch email in the
> maling list archive to your proposal.

I sent it as a [RFC] patch to the mailing list and included it in the proposal.

<https://public-inbox.org/git/20180324182313.13705-1-ungureanupaulsebastian@gmail.com>

Show 6 quoted lines

> It could be useful if you described a bit more how you could (re)use
> the work that has already been made.
> 
> In the rest of your proposal it looks like you are starting from
> scratch, which is a bit strange if a lot of progress has already been
> made.

The patches about converting ‘git stash’ are a great starting point and I will definitely use them. Each time I start converting a new command I will first take a look at what other patches there are, evaluate them and if I consider the patch to be good enough I will continue my work on top of that patch. Otherwise, I will start from scratch and that patch will only serve for comparison.

One other resource that is of great importance is git itself. I can learn how a builtin is structured and how it is built by considering examples the other Git commands. Having a similar coding standard used, the codebase will be homogeneous and hopefully easier to maintain.

Another important resource are commands that are Google Summer of Code projects from previous years (2016 and 2017) which had as purpose to convert “git bisect” and “git submodule”. I can always take a look at the patches they submitted and read their code reviews to avoid making same mistakes they did.

Show 9 quoted lines

> It is probably better especially for reviewers and more common to work
> in small batches, for example to send a patch series converting a few
> related commands, then to start working on the next small batch of
> commands in another patch series while improving the first patch
> series according to reviews.
> 
> Also we ask GSoC students to communicate publicly every week about
> their project for example by writing a blg post or sending a report to
> the mailing list.

Noted.

Show 15 quoted lines

>> ## Git contributions
>> ------------------------------------------
>> My first contribution to Git was to help making “git tag --contains
>> <id>” les chatty if <id> is invalid. Looking over the list of available
>> microprojects, there were a couple of microprojects which got my
>> attention, but I picked this up because it seemed to be a long-standing
>> bug (I noticed it was approached by students in 2016, 2017 and now in
>> 2018). Thanks to the code reviews from the mailing list, after a few
>> iterations I succeeded in coming up with a patch that not only fixed
>> the mentioned problem, but also a few others that were having the same
>> cause.
>>
>> It got merged in the proposed updates branch.
> 
> What is its status in Junio's "What's cooking in Git" emails?

It is now in the ‘next’ branch of the Git repository.

I updated the proposal, in which I included these ideas and some additional examples. Thank you a lot!

Best regards, Paul Ungureanu