On Sat, Mar 24, 2018 at 8:31 PM, Paul-Sebastian Ungureanu <ungureanupaulsebast...@gmail.com> wrote: > 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.
I was thinking about things like memory leaks that tests cannot easily spot. >>> * 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. Good. >>> ## 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-ungureanupaulsebast...@gmail.com> Nice. >> 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. Nice. >> 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! Ok. Feel free to resend another version of your proposal. Thanks.
