Hi Andrei, and thanks for trying to help improve git-p4! :-)
2011/10/14 Andrei Warkentin <andreiw@xxxxxxxxxx>:
> Many users of p4/sd use changelists for review, regression
> tests and batch builds, thus changes are almost never directly
> submitted.
Just out of curiosity... what is 'sd'?
> This new config option lets a 'p4 change -i' run instead of
> the 'p4 submit -i'.
Well... I have to say that I'm not crazy about this patch... I don't
think it is very elegant to have a config flag that says that "when
the user says 'git p4 submit', then don't submit, but do something
else instead".
I would much rather have made a patch to introduce some new command
like 'git p4 change'.
> Signed-off-by: Andrei Warkentin <andreiw@xxxxxxxxxx>
> ---
> contrib/fast-import/git-p4 | 16 ++++++++++++----
> contrib/fast-import/git-p4.txt | 10 ++++++++++
> 2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
> index 2f7b270..19c295b 100755
> --- a/contrib/fast-import/git-p4
> +++ b/contrib/fast-import/git-p4
> @@ -959,7 +959,10 @@ class P4Submit(Command, P4UserMap):
> submitTemplate = message[:message.index(separatorLine)]
> if self.isWindows:
> submitTemplate = submitTemplate.replace("\r\n", "\n")
> - p4_write_pipe("submit -i", submitTemplate)
> + if gitConfig("git-p4.changeOnSubmit"):
> + p4_write_pipe("change -i", submitTemplate)
> + else:
> + p4_write_pipe("subadasdmit -i", submitTemplate)
... 'subadasdmit'? Did some debug/test code sneak in to your patch?
>
> if self.preserveUser:
> if p4User:
> @@ -981,9 +984,14 @@ class P4Submit(Command, P4UserMap):
> file = open(fileName, "w+")
> file.write(self.prepareLogMessage(template, logMessage))
> file.close()
> - print ("Perforce submit template written as %s. "
> - + "Please review/edit and then use p4 submit -i < %s to submit directly!"
> - % (fileName, fileName))
> + if gitConfig("git-p4.changeOnSubmit"):
> + print ("Perforce submit template written as %s. "
> + + "Please review/edit and then use p4 change -i < %s to create changelist!"
> + % (fileName, fileName))
> + else:
> + print ("Perforce submit template written as %s. "
> + + "Please review/edit and then use p4 submit -i < %s to submit directly!"
> + % (fileName, fileName))
>
> def run(self, args):
> if len(args) == 0:
> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
> index 52003ae..3a3a815 100644
> --- a/contrib/fast-import/git-p4.txt
> +++ b/contrib/fast-import/git-p4.txt
> @@ -180,6 +180,16 @@ git-p4.allowSubmit
>
> git config [--global] git-p4.allowSubmit false
>
> +git-p4.changeOnSubmit
> +
> + git config [--global] git-p4.changeOnSubmit false
> +
> +Most places using p4/sourcedepot don't actually want you submit
> +changes directly, and changelists are used to do regression testing,
> +batch builds and review, hence, by setting this parameter to
> +true you acknowledge you end up creating a changelist which you
> +must then manually commit.
> +
It might be just me, but I never heard of 'sourcedepot' before this
patch... Google tells me that it might be some old version of p4 that
Microsoft used many many years ago. If that's the case, maybe it
doesn't add much value to talk about it in git-p4 docs.. (??)
And you claim 'most places don't want people to submit directly'. I'm
not sure I agree with that, and anyway it seems like the git-p4 docs
should be phrased more neutral than that.
My advice would be, as I mentioned earlier, to rework this into a
patch introducing a separate command 'git p4 change' instead of this
config flag.
Have a good one!
Tor Arvid
> git-p4.syncFromOrigin
>
> A useful setup may be that you have a periodically updated git repository
> --
> 1.7.4.1
>
> --
> 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
>
--
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