On Sat, 18 Jun 2016 20:37:06 +0100
Sergei Trofimovich <slyich@gmail.com> wrote:

> On Sat, 18 Jun 2016 11:02:53 -0700
> Brian Dolbec <dolsen@gentoo.org> wrote:
> 
> > > +		if runtime:
> > > +			try:
> > > +				# to find use of ':=' in '||' we
> > > preserve
> > > +				# tree structure of dependencies
> > > +				hier_atoms =
> > > portage.dep.use_reduce(
> > > +					mydepstr,
> > > +					flat=False,
> > > +					matchall=1,
> > > +
> > > is_valid_flag=pkg.iuse.is_valid_flag,
> > > +					opconvert=True,
> > > +					token_class=token_class)
> > > +			except
> > > portage.exception.InvalidDependString as e:
> > > +				hier_atoms = None
> > > +				badsyntax.append(str(e))
> > > +			check_slotop(hier_atoms, mytype,
> > > qatracker, ebuild.relative_path)    
> > 
> > 
> > Is there a special reason this code can not be part of the
> > check_slotop()?  All it has is the one statement calling it's
> > internal recursive function.  Then all this code would would be
> > replaced here by the one check_slotop() which adds to badsyntax.  
> 
> I don't have special preference. I've followed example of what a
> check few lines before does to call check_missingslot(). I've sent a
> V2 patch w/o pulling it off to a separate helper.
> 
> If you think it should be better moved completely to a separate
> helper I'll move it there. As you see I have a very weak python fu.
> 
> If there is examples of better ways to pass that large context of
> variables please point me to it :)
> 

OK, V2 is better, but not quite what I had thought of.

The original check_sloptop() had 2 items, the _traverse_tree() and the
one staement calling it.  So, yes it is better that in that it doesn't
have the needless sub function.  But the _depend_checks() is getting
quite long already.  The new code you add in there makes it even longer
and adds even more branching.  It can lead to things being more
complicated for future changes.  What I meant was to move most of that
additional code into the pretty unused check_sloptop() and have it call
the _traverse_tree() from there.  That would simplify the
depends_check() and move the check_sloptop specific code to that
function.  Then the _traverse_tree sub-function would be utilized with
the check_slotop() better utilized.

 so:

+		if runtime:
+                   check_slotop(...)

would be all that was added to _depends_check() directly.

Zac, what do you think?
-- 
Brian Dolbec <dolsen>