From: Pratyush Yadav <me@yadavpratyush.com>
To: Birger Skogeng Pedersen <birger.sp@gmail.com>
Cc: Git List <git@vger.kernel.org>,
Bert Wesarg <bert.wesarg@googlemail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v4] git-gui: add hotkey to toggle "Amend Last Commit"
Date: Sat, 14 Sep 2019 03:41:19 +0530 [thread overview]
Message-ID: <20190913221119.552477ff3cl2q5pc@yadavpratyush.com> (raw)
In-Reply-To: <CAGr--=JhBYmYCJNNm8DyL+MKU0V0V-cwzH4WABX-dvE+uXNwDw@mail.gmail.com>
+Cc Junio so you know what development model I'm using, and comment with
your thoughts (if you want to).
On 13/09/19 11:32PM, Birger Skogeng Pedersen wrote:
> On Fri, Sep 13, 2019 at 4:37 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > Hi Birger,
> >
> > I'm afraid you are working on an older version of this patch. You should
> > be re-rolling [0], which works well with Bert's "amend check button"
> > change.
> >
> > [0] https://public-inbox.org/git/b82a00441ff1a6a9cea3fd235c1c33729ec31b71.1567713659.git.bert.wesarg@googlemail.com/
>
> Forgive me, I get a little bit confused. Should my patch be based on
> "next" or "master" branch?
For git-gui, ignore "next" for now. I considered using a model similar
to Git where patches first get queued to "next" (or "pu" depending on
how finished they are). And then after some time letting people use
them, they are merged to "master" which eventually goes in the release.
But this seems to be too complicated to me without any clear benefit.
I think for now using just "master" for git-gui should be fine, since I
won't directly release git-gui. Instead I'll periodically ask Junio to
pull changes from my master. This will be our "release". So essentially
my "master" for now acts as a place for people involved in development
to test out all the changes, and then the rest of the world gets the new
version along with Git's release.
(Junio, you have done this for much longer than I have, so is there a
major problem with my workflow?)
If all this seems too complicated, just work on top of my "master". The
rest of it is mostly my problem.
> Also, is it an issue that this patch won't work unless you merge
> Bert's 1/2 patch[0]?
Your patch is dependent on Bert's patch. This means I will have to merge
his patch first, and then yours. And that makes complete sense. That's
how dependent changes should work. So no, it is not an issue that this
patch won't work unless I merge Bert's patch first.
So while my advice above was to work on top of "master", that does not
apply in this case since your patch is dependent on someone's patch
which isn't in master yet. So in this specific case, you should base
your patch on top of Bert's. Otherwise there are two problems:
- Your patch in and of itself makes little sense, it probably would
crash or do some unexpected stuff. This hurts people doing a bisect,
where if they land on your patch stuff is broken, and they have to
manually move a commit up or down to continue.
- Your patch will textually conflict with Bert's. That means I'd first
merge Bert's patch since yours doesn't work without his. But then when
I merge yours, I'd get a nasty merge conflict that is not easy to
resolve and leaves chances for a subtle bug.
> Your feedback cannot be too specific, I want to learn how to do this
> properly :-)
I'm not sure how well I explained this, so feel free to ask more
questions if I didn't explain something properly :).
> [0] https://public-inbox.org/git/ab1f68cc8552e405c9d04622be1e728ab81bda17.1567713659.git.bert.wesarg@googlemail.com/
--
Regards,
Pratyush Yadav
next prev parent reply other threads:[~2019-09-13 22:11 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 20:09 [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Bert Wesarg
2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg
2019-09-11 20:55 ` Pratyush Yadav
2019-09-12 6:05 ` Birger Skogeng Pedersen
2019-09-12 16:29 ` Pratyush Yadav
2019-09-12 18:41 ` [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" Birger Skogeng Pedersen
2019-09-13 14:37 ` Pratyush Yadav
2019-09-13 21:11 ` [PATCH v4] " Birger Skogeng Pedersen
2019-09-13 21:32 ` Birger Skogeng Pedersen
2019-09-13 22:11 ` Pratyush Yadav [this message]
2019-09-14 9:07 ` Birger Skogeng Pedersen
2019-09-14 9:18 ` [PATCH v5] " Birger Skogeng Pedersen
2019-09-14 17:48 ` Pratyush Yadav
2019-09-16 12:23 ` Birger Skogeng Pedersen
2019-09-12 21:34 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Marc Branchaud
2019-09-12 22:23 ` Philip Oakley
2019-09-13 7:50 ` Birger Skogeng Pedersen
2019-09-13 13:55 ` Marc Branchaud
2019-09-13 17:47 ` Pratyush Yadav
2019-09-05 20:33 ` [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Pratyush Yadav
2019-09-11 20:15 ` Pratyush Yadav
2019-09-12 19:35 ` Bert Wesarg
2019-09-13 17:14 ` Pratyush Yadav
2019-09-12 19:39 ` Bert Wesarg
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=20190913221119.552477ff3cl2q5pc@yadavpratyush.com \
--to=me@yadavpratyush.com \
--cc=bert.wesarg@googlemail.com \
--cc=birger.sp@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).