From: Pete Wyckoff <pw@padd•com>
To: Andrei Warkentin <awarkentin@vmware•com>
Cc: git@vger•kernel.org, Andrei Warkentin <andreiw@vmware•com>,
Tor Arvid Lund <torarvid@gmail•com>,
Luke Diamand <luke@diamand•org>,
gitster@pobox•com
Subject: Re: [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit'.
Date: Mon, 17 Oct 2011 18:32:02 -0400 [thread overview]
Message-ID: <20111017223202.GA1834@arf.padd.com> (raw)
In-Reply-To: <4E9C799E.70700@diamand.org>
luke@diamand•org wrote on Mon, 17 Oct 2011 19:53 +0100:
> On 17/10/11 17:18, Andrei Warkentin wrote:
> >Hi,
> >
> >----- Original Message -----
> >Anyway, the other suggestion I had was to create a new command
> >instead of overriding behaviour of an existing one. Of course,
> >copy-pasting P4Submit into P4Change is silly, so...
> >
> >How about something like this?
> >
> >The commands dict maps command name to class and optional dict passed to cmd.run(). That way 'change'
> >can really mean P4Submit with an extra parameter not to submit but to do a changelist instead. The
> >reason why I initially made the config flag was because I didn't want to copy-paste P4Submit into P4Change.
> >
> >commands = {
> > "debug" : [ P4Debug, {} ]
> > "submit" : [ P4Submit, { "doChange" : 0 } ]
> > "commit" : [ P4Submit, { "doChange" : 0 } ]
> > "change" : [ P4Submit, { "doChange" : 1 } ]
> > "sync" : [ P4Sync, {} ],
> > "rebase" : [ P4Rebase, {} ],
> > "clone" : [ P4Clone, {} ],
> > "rollback" : [ P4RollBack, {} ],
> > "branches" : [ P4Branches, {} ]
> >}
> >
> >Thanks for the review,
> >A
> >
>
> Sounds plausible to me. The alternative would be a command line
> parameter, although that could get annoying and error prone,
> especially as you can't easily unsubmit a perforce change.
This seems like a useful thing to do, but needs some care.
Git can have multiple commits outstanding that touch the same
file, but p4 cannot really have multiple pending changes in the
same workspace that touch the same file.
If you call "git-p4 change", it would build a p4 change for each
of those commits. If the commits happen to touch the same file,
the changes get rearranged as far as p4 is concerned so that all
changes to a given file are lumped in the first change that sees
the file. This is highly counterintuitive from a git mindset.
The most restrictive implementation would have to:
1. ensure no pending changes in the P4 clientPath
2. ensure number of commits ("git rev-list") is 1
You could be more permissive, allowing multiple pending changes
if the file sets do not conflict. In that case, the first test
would look at the files in pending changes and allow the
operation if they did not intersect with files in origin..master.
The second would make sure that each file appears in no more than
1 commit in origin..master.
Also make sure this works with preserveUser. Not sure if an
unsubmitted change can be handled the same way.
Because it feels like a delicate operation that could have big
negative consequences, this needs a few unit tests.
For the code structure, I'd like to see a proper subclass instead
of the dictionary idea. Something like, e.g.:
class P4Submit(...):
def __init__(self, change_only=0)
...
self.change_only = change_only
class P4Change(P4Submit):
def __init__(self):
P4Submit.__init__(self, change_only=1)
Sorry this is looking so difficult now.
-- Pete
next prev parent reply other threads:[~2011-10-17 22:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-14 21:51 [PATCH] Git-p4: git-p4.changeOnSubmit to do 'change' instead of 'submit' Andrei Warkentin
2011-10-14 22:31 ` Tor Arvid Lund
2011-10-14 22:55 ` Andrei Warkentin
2011-10-15 20:10 ` Luke Diamand
2011-10-17 16:18 ` Andrei Warkentin
2011-10-17 18:53 ` Luke Diamand
2011-10-17 22:32 ` Pete Wyckoff [this message]
2011-10-17 22:37 ` Andrei Warkentin
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=20111017223202.GA1834@arf.padd.com \
--to=pw@padd$(echo .)com \
--cc=andreiw@vmware$(echo .)com \
--cc=awarkentin@vmware$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=luke@diamand$(echo .)org \
--cc=torarvid@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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