luke@xxxxxxxxxxx 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html