Re: [PATCHv1 4/6] git-p4: create helper function importRevisions()

From: Luke Diamand <luke@diamand.org>
Date: Jan 29, 2020, 18:36 UTC
Message-ID: <CAE5ih785Nv584RU-xEQSqrcuGiUU2zmjRrJ=ghtFX7_7EuiDnw@mail.gmail.com>
In-Reply-To: <CAPig+cRx3hG64nuDie69o_gdX39F=sR6D8LyA7J1rCErgu0aMA@mail.gmail.com>

On Wed, 29 Jan 2020 at 15:00, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 20 quoted lines

>
> On Wed, Jan 29, 2020 at 6:13 AM Luke Diamand <luke@diamand.org> wrote:
> > This makes it easier to try/catch around this block of code to ensure
> > cleanup following p4 failures is handled properly.
> >
> > Signed-off-by: Luke Diamand <luke@diamand.org>
> > ---
> > diff --git a/git-p4.py b/git-p4.py
> > @@ -3555,6 +3555,73 @@ def importHeadRevision(self, revision):
> > +    def importRevisions(self, args, branch_arg_given):
> > +        if len(self.changesFile) > 0:
> > +            output = open(self.changesFile).readlines()
>
> Not a new problem (since this code is merely being relocated), but is
> this leaking the open file? Should there be an accompanying close()
> somewhere?
>
>     f = open(self.changesFile)
>     output = f.readlines()
>     close(f)

with open(self.changesFile) as f:
    output = f.readlines()

I can put something like that in either as a followup, or the next re-roll.

>
> or something.