git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
To: newren@gmail.com
Cc: Johannes.Schindelin@gmx.de, artagnon@gmail.com,
	christian.couder@gmail.com, git@vger.kernel.org,
	rohit.ashiwal265@gmail.com, s-beyer@gmx.net,
	t.gummerer@gmail.com, rafa.almas@gmail.com
Subject: Re: [GSoC][RFC] Proposal: Improve consistency of sequencer commands
Date: Sun, 24 Mar 2019 07:15:30 +0530
Message-ID: <20190324014530.8218-1-rohit.ashiwal265@gmail.com> (raw)
In-Reply-To: <CABPp-BEdgSHGTkUcg_UDRu50Ag+cCqigmvU4_JaRZRnDpgWcdA@mail.gmail.com>

Hi Elijah!

On Sat, 23 Mar 2019 18:07:17 -0700 Elijah Newren <newren@gmail.com> wrote:
> Hi Rohit!
> 
> On Fri, Mar 22, 2019 at 8:12 AM Rohit Ashiwal
> <rohit.ashiwal265@gmail.com> wrote:
> > PS: Point one is missing in the timeline from the ideas page[0], can someone
> >     explain what exactly it wants?
> 
> I don't understand the question; could you restate it?

I was talking about this point: " The suggestion to fix an interrupted rebase-i
or cherry-pick due to a commit that became empty via git reset HEAD (in builtin/commit.c)
instead of git rebase --skip or git cherry-pick --skip ranges from annoying to confusing.".

> > Points to work on:
> > ------------------
> >     - Add `git cherry-pick --skip`
> 
> I'd reword this section as 'Consistently suggest --skip for operations
> that have such a concept'.[1]

Alright! I'll correct this in comming revisions.

> >     - [Bonus] Deprecate am-based rebases
> >     - [Bonus] Make a flag to allow rebase to rewrite commit messages that
> >           refer to older commits that were also rebased
> 
> I'd reorder these two.  I suspect the second won't be too hard and
> will provide a new user-visible feature, while the former will
> hopefully not be visible to users; if the former has more than
> cosmetic differences visible to user, it might transform the problem
> into more of a social problem than a technical one or just make into
> something we can't do.

There is no "order" in these points, just a rough TODO list, but I get your point here.
 
> > Proposed Timeline
> > -----------------
> >     + Community Bonding (May 6th - May 26th):
> >         - Introduction to community
> >         - Get familiar with the workflow
> >         - Study and understand the workflow and implementation of the project in detail
> >
> >     + Phase 1  (May 27th - June 23rd):
> >         - Start with implementing `git cherry-pick --skip`
> >         - Write new tests for the just introduced flag(s)
> >         - Analyse the requirements and differences of am-based and other rebases flags
> 
> Writing or finding tests to trigger all the --skip codepaths might be
> the biggest part of this phase.  Implementing `git cherry-pick --skip`
> just involves making it run the code that `git reset` invokes.  The
> you change the error message to reference `<command> --skip` instead
> of `git reset`.  What you're calling phase 1 here isn't quite
> microproject sized, but it should be relatively quick and easy; I'd
> plan to spend much more of your time on phase 2.
> 
> >     + Phase 2  (June 24th - July 21st):
> >         - Introduce flags of am-based rebases to other kinds.
> >         - Add tests for the same.
> 
> You should probably mention the individual cases from "INCOMPATIBLE
> FLAGS" of the git rebase manpage.  Also, some advice for order of
> tackling these: I think you should probably do --ignore-whitespace
> first; my guess is that one is the easiest.  Close up would be
> --committer-date-is-author-date and --ignore-date.  Re-reading, I'm
> not sure -C even makes sense at all; it might be that the solution is
> just accepting the flag and ignoring it, or perhaps it remains the one
> flag the interactive backend won't support, or maybe there is
> something that makes sense to be done.  There'd need to be a little
> investigation for that one, but it might turn out simple too.  The
> --whitespace={nowarn|warn|fix|error|error-all} flag will be the
> kicker.  I don't know how long that one will take, but I'm certain
> it's harder than the other flags and it might conceivably take up most
> the summer or even extend beyond.
> 
> >     + Phase 3  (July 22th - August 19th):
> >         - Act on [Bonus] features
> >         - Documentation
> >         - Clean up tasks
> 
> I'd prefer that Documentation updates were made as you went; you'll
> particularly need to look at Documentation/git-cherry-pick.txt and
> Documentation/rebase.txt, especially the "INCOMPATIBLE OPTIONS" and
> "BEHAVIORAL DIFFERENCES" sections of the latter.

Thanks for advice, yes, of course, the documentation and implementation will go
hand in hand.
 
> Also, as far as timing goes, the rewriting of commit messages seems
> relatively straightforward; you may want to consider doing it before
> the --whitespace flag (despite the fact that I originally suggested it
> as a bonus item).  Deprecating am-based rebases, on the other hand, is
> a bit of a wildcard.  It depends on Phase 2 being completed so it
> definitely makes sense to be last.  If phase 2 is complete, it's
> conceivable that deprecating am-based rebases only takes a little more
> work, but it might expand to use up a lot of time.
> 
> > Relevant Work
> > =============
> > Dscho and I had a talk on how a non-am backend should implement `git rebase
> > --whitespace=fix`, which he warned may become a large project (as it turns
> > out it is a sub-task in one of the proposed ideas[0]), we were trying to
> > integrate this on git-for-windows first.
> > Keeping warning in mind, I discussed this project with Rafael and he suggested
> > (with a little bit uncertainty in mind) that I should work on implementing
> > a git-diff flag that generates a patch that when applied, will remove whitespace
> > errors which I am currently working on.
> 
> It's awesome that you're looking in to this, but it may make more
> sense to knock out the easy parts of this project first.  That way the
> project gets some value out of your work for sure, you gain confidence
> and familiarity with the codebase, and then you can tackle the more
> difficult items.  Of course, if you're just exploring to learn what's
> possible in order to write the proposal, that's fine, I just think
> once you start on this project, it'd make more sense to do the easier
> ones first.

Yes, I'm looking into the code to get some clear vision.

> Hope that helps,
Yes! The vision in now clearer. Thanks Elijah. :)
> Elijah

Thanks for the review
Rohit

  reply	other threads:[~2019-03-24  1:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-24 10:05 [GSoC] Introduction Rohit Ashiwal
2019-02-24 14:47 ` Johannes Schindelin
2019-02-25  6:50 ` Christian Couder
2019-02-25 11:35   ` Rohit Ashiwal
2019-02-25 20:21     ` Christian Couder
2019-02-25 21:09       ` Eric Sunshine
2019-03-22 15:11 ` [GSoC][RFC] Proposal: Improve consistency of sequencer commands Rohit Ashiwal
2019-03-23 22:17   ` Christian Couder
2019-03-24  1:21     ` Rohit Ashiwal
2019-03-24  1:07   ` Elijah Newren
2019-03-24  1:45     ` Rohit Ashiwal [this message]
2019-03-29 22:32 ` [GSoC][RFC v2] " Rohit Ashiwal
2019-03-29 23:25   ` Elijah Newren
2019-03-29 23:34     ` Rohit Ashiwal
2019-03-30  0:38       ` Elijah Newren
2019-03-30  8:48         ` Rohit Ashiwal
2019-03-30 17:13           ` Elijah Newren
2019-03-30  7:16   ` Christian Couder
2019-03-30 17:12     ` Elijah Newren
2019-04-05 21:31 ` [GSoC][RFC v3] Proposal: " Rohit Ashiwal
2019-04-07  7:15   ` Christian Couder
2019-04-07 12:16     ` Rohit Ashiwal
2019-04-07 23:07       ` 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190324014530.8218-1-rohit.ashiwal265@gmail.com \
    --to=rohit.ashiwal265@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=artagnon@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=rafa.almas@gmail.com \
    --cc=s-beyer@gmx.net \
    --cc=t.gummerer@gmail.com \
    --subject='Re: [GSoC][RFC] Proposal: Improve consistency of sequencer commands' \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git