From: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [GSoC] Convert “git stash” to builtin proposal
Date: Sat, 24 Mar 2018 21:31:14 +0200 [thread overview]
Message-ID: <6603149f-776a-fde8-5d11-a7d9d6d37e96@gmail.com> (raw)
In-Reply-To: <CAP8UFD3bRaPke8MvubZ3+v6RrY7K7Peip1dpQ2LG9kxKoXcmbw@mail.gmail.com>
On 23.03.2018 19:11, Christian Couder wrote:
>> * 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.
>> * 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.
>> ## 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>
> 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.
> 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.
>> ## 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
next prev parent reply other threads:[~2018-03-24 19:31 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-20 20:09 [GSoC] Convert “git stash” to builtin proposal Paul-Sebastian Ungureanu
2018-03-20 22:08 ` Christian Couder
2018-03-22 23:15 ` Paul-Sebastian Ungureanu
2018-03-23 17:06 ` Johannes Schindelin
2018-03-25 14:32 ` Paul-Sebastian Ungureanu
2018-03-23 17:11 ` Christian Couder
2018-03-24 19:31 ` Paul-Sebastian Ungureanu [this message]
2018-03-25 9:46 ` Christian Couder
2018-03-25 14:38 ` Paul-Sebastian Ungureanu
2018-03-26 22:04 ` Johannes Schindelin
2018-03-27 6:14 ` Christian Couder
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=6603149f-776a-fde8-5d11-a7d9d6d37e96@gmail.com \
--to=ungureanupaulsebastian@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
/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.
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 NNTP newsgroup(s).