Re: [PATCHv1 1/1] git-p4: better error reporting when p4 fails
On Tue, Jun 5, 2018 at 6:59 AM Luke Diamand wrote: > On 5 June 2018 at 11:49, Merland Romain wrote: > >> +# now check that we can actually talk to the server > >> +global p4_access_checked > >> +if not p4_access_checked: > >> +p4_access_checked = True > >> +p4_check_access() > >> + > > > > Just switch the 2 lines 'p4_access_checked = True' and 'p4_check_access()' > > It seems to me more logical > > Like this: > > +p4_check_access() > +p4_access_checked = True > > You need to set p4_access_checked first so that it doesn't go and try > to check the p4 access before running "p4 login -s", which would then > get stuck forever. Such subtlety may deserve an in-code comment.
Re: [PATCHv1 1/1] git-p4: better error reporting when p4 fails
On 5 June 2018 at 11:49, Merland Romain wrote: > >> @@ -91,6 +93,13 @@ def p4_build_cmd(cmd): >> real_cmd = ' '.join(real_cmd) + ' ' + cmd >> else: >> real_cmd += cmd >> + >> +# now check that we can actually talk to the server >> +global p4_access_checked >> +if not p4_access_checked: >> +p4_access_checked = True >> +p4_check_access() >> + > > Just switch the 2 lines 'p4_access_checked = True' and 'p4_check_access()' > It seems to me more logical Like this: +p4_check_access() +p4_access_checked = True You need to set p4_access_checked first so that it doesn't go and try to check the p4 access before running "p4 login -s", which would then get stuck forever. > > Romain