1 |
On Sat, 18 Jun 2016 20:37:06 +0100 |
2 |
Sergei Trofimovich <slyich@×××××.com> wrote: |
3 |
|
4 |
> On Sat, 18 Jun 2016 11:02:53 -0700 |
5 |
> Brian Dolbec <dolsen@g.o> wrote: |
6 |
> |
7 |
> > > + if runtime: |
8 |
> > > + try: |
9 |
> > > + # to find use of ':=' in '||' we |
10 |
> > > preserve |
11 |
> > > + # tree structure of dependencies |
12 |
> > > + hier_atoms = |
13 |
> > > portage.dep.use_reduce( |
14 |
> > > + mydepstr, |
15 |
> > > + flat=False, |
16 |
> > > + matchall=1, |
17 |
> > > + |
18 |
> > > is_valid_flag=pkg.iuse.is_valid_flag, |
19 |
> > > + opconvert=True, |
20 |
> > > + token_class=token_class) |
21 |
> > > + except |
22 |
> > > portage.exception.InvalidDependString as e: |
23 |
> > > + hier_atoms = None |
24 |
> > > + badsyntax.append(str(e)) |
25 |
> > > + check_slotop(hier_atoms, mytype, |
26 |
> > > qatracker, ebuild.relative_path) |
27 |
> > |
28 |
> > |
29 |
> > Is there a special reason this code can not be part of the |
30 |
> > check_slotop()? All it has is the one statement calling it's |
31 |
> > internal recursive function. Then all this code would would be |
32 |
> > replaced here by the one check_slotop() which adds to badsyntax. |
33 |
> |
34 |
> I don't have special preference. I've followed example of what a |
35 |
> check few lines before does to call check_missingslot(). I've sent a |
36 |
> V2 patch w/o pulling it off to a separate helper. |
37 |
> |
38 |
> If you think it should be better moved completely to a separate |
39 |
> helper I'll move it there. As you see I have a very weak python fu. |
40 |
> |
41 |
> If there is examples of better ways to pass that large context of |
42 |
> variables please point me to it :) |
43 |
> |
44 |
|
45 |
OK, V2 is better, but not quite what I had thought of. |
46 |
|
47 |
The original check_sloptop() had 2 items, the _traverse_tree() and the |
48 |
one staement calling it. So, yes it is better that in that it doesn't |
49 |
have the needless sub function. But the _depend_checks() is getting |
50 |
quite long already. The new code you add in there makes it even longer |
51 |
and adds even more branching. It can lead to things being more |
52 |
complicated for future changes. What I meant was to move most of that |
53 |
additional code into the pretty unused check_sloptop() and have it call |
54 |
the _traverse_tree() from there. That would simplify the |
55 |
depends_check() and move the check_sloptop specific code to that |
56 |
function. Then the _traverse_tree sub-function would be utilized with |
57 |
the check_slotop() better utilized. |
58 |
|
59 |
so: |
60 |
|
61 |
+ if runtime: |
62 |
+ check_slotop(...) |
63 |
|
64 |
would be all that was added to _depends_check() directly. |
65 |
|
66 |
Zac, what do you think? |
67 |
-- |
68 |
Brian Dolbec <dolsen> |