On Tue, Jun 5, 2018 at 6:59 AM Luke Diamand <luke@xxxxxxxxxxx> wrote: > On 5 June 2018 at 11:49, Merland Romain <merlorom@xxxxxxxx> 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.