Re: svn commit: r1864256 - /subversion/site/tools/upcoming.py

On Tue, Aug 6, 2019 at 9:41 AM Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:

> Greg Stein wrote on Tue, 06 Aug 2019 07:58 +00:00:
> > On Fri, Aug 2, 2019 at 12:53 PM <danielsh_at_apache.org> wrote:
> > >...
> > > +++ subversion/site/tools/upcoming.py Fri Aug 2 17:53:38 2019
> > >...
> > > +def get_reference_version():
> > > + "Return the version to use as the oldest end of the 'svn log'
> output to generate."
> > > + def _is_working_copy():
> > > + return os.path.exists('subversion/include/svn_version.h')
> > > + if _is_working_copy():
> >
> > Why a local func instead of just using os.path.exists() in the 'if'
> > statement? If for doc purposes, then I think a comment would suffice.
>
> Yes, for doc purposes. How would a comment be better than a one-line
> helper function?
>

For me, it was the complexity. "Oh! A local function. What is this gonna be
used for? ... oh." Local functions are a pretty high-level Python Fu. It
kinda stood out to me.

It is a rather complicated commentary. I'd think a simpler approach:
# are we looking at a local working copy?

Cheers,
-g
Received on 2019-08-07 07:28:33 CEST