All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alban Gruin <alban.gruin@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git <git@vger.kernel.org>, Christian Couder <christian.couder@gmail.com>
Subject: Re: [RFC][GSoC] Project proposal: convert interactive rebase to C
Date: Wed, 21 Mar 2018 12:04:28 +0100	[thread overview]
Message-ID: <46112353.MVnaADNoVi@andromeda> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1803201720090.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

Hi Johannes,

Le mardi 20 mars 2018 17:29:28 CET, vous avez écrit :
> > Weeks 1 & 2 — May 14, 2018 – May 28, 2018
> > First, I would refactor --preserve-merges in its own shell script, as
> > described in Dscho’s email.
> 
> Could you go into detail a bit here? Like, describe what parts of the
> git-rebase--interactive.sh script would need to be duplicated, which ones
> would be truly moved, etc
> 

It would lead to duplicate a good chunk of git_rebase__interactive(), 
apparently. The moved parts would be everything in `if test t = 
"$preserve_merges"; then …; fi` statements. That is, about 50 lines of shell 
code.

Judging by that, beginning by that is probably not the right thing to do. 
Also, somebody is already working on that[1]. 

> > Weeks 3 & 4 — May 18, 2018 – June 11, 2018
> > Then, I would start to rewrite git-rebase--interactive, and get rid of
> > git-
> > rebase--helper.
> 
> I think this is a bit premature, as the rebase--helper would not only
> serve the --interactive part, but in later conversions also --am and
> --merge, and only in the very end, when git-rebase.sh gets converted,
> would we be able to simply rename the rebase--helper to rebase.
> 

Yes, Christian Couder told me that it would not be a good methodology too.

> Also, I have a hunch that there is actually almost nothing left to rewrite
> after my sequencer improvements that made it into Git v2.13.0, together
> with the upcoming changes (which are on top of the --recreate-merges patch
> series, hence I did not send them to the mailing list yet) in
> https://github.com/dscho/git/commit/c261f17a4a3e

One year ago, you said[2] that converting this script "will fill up 3 month, 
very easily". Is this not accurate anymore?

> 
> So I would like to see more details here... ;-)

Yep, I’m working on that. 

> > Weeks 5 to 9 — June 11, 2018 – July 15, 2018
> > During this period, I would continue to rewrite git-rebase--interactive.
> 
> It would be good if the proposal said what parts of the conversion are
> tricky, to merit spending a month on them.
> 
> > Weeks 10 & 11 — July 16, 2018 – July 29, 2018
> > In the second half of July, I would look for bugs in the new code, test
> > it,
> > and improve its coverage.
> 
> As I mentioned in a related mail, the test suite coverage would be most
> sensibly extended *before* starting to rewrite code in C, as it helps
> catching bugs early and avoids having to merge buggy code that needs to be
> fixed immediately.

Makes sense.

> 
> > Weeks 12 — July 30, 2018 – August 5, 2018
> > In the last week, I would polish the code where needed, in order to
> > improve for performance or to make the code more readable.
> 
> Thank you for sharing this draft with us!
> Johannes

I’ll send a new draft as soon as possible (hopefully this afternoon).

Thank you for your enthousiasm :)
Alban

[1] https://public-inbox.org/git/20180320204507.12623-1-wink@saville.com/
[2] https://public-inbox.org/git/alpine.DEB.
2.20.1703231827060.3767@virtualbox/



  reply	other threads:[~2018-03-21 11:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-17 19:14 [RFC][GSoC] Project proposal: convert interactive rebase to C Alban Gruin
2018-03-17 20:29 ` Christian Couder
2018-03-20 16:29 ` Johannes Schindelin
2018-03-21 11:04   ` Alban Gruin [this message]
2018-03-21 23:51     ` Johannes Schindelin
2018-03-22 22:03 ` Alban Gruin
2018-03-24  7:43   ` Christian Couder
2018-03-25  0:43   ` [RFC v3][GSoC] " Alban Gruin

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=46112353.MVnaADNoVi@andromeda \
    --to=alban.gruin@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.