| 1 |
On Sun, 19 Jan 2014 18:43:46 -0800 |
| 2 |
Alec Warner <antarus@g.o> wrote: |
| 3 |
|
| 4 |
> It is certainly weird (as we discussed on IRC.) I've never seen |
| 5 |
> anyone do it in any codebase I liked. |
| 6 |
|
| 7 |
My backlog was limited so I didn't catch that discussion, feel free to |
| 8 |
share the log; I've since increased it. There's a lot more talk than my |
| 9 |
defaults on IRC (as well as here on the mailing list). :) |
| 10 |
|
| 11 |
On a side note, "I liked" seems a too subjective way to review patches. |
| 12 |
|
| 13 |
> One of the problems is that it isn't immutable, so that earlier |
| 14 |
> callers can mess with later callers. That is not possible in vapier's |
| 15 |
> proposal, as the attributes are hidden in the function code and are |
| 16 |
> not visible to callers. |
| 17 |
|
| 18 |
True, but do you have a better suggestion? (Not the one below) |
| 19 |
|
| 20 |
From a quick lookup Python seems to not really provide a clean |
| 21 |
immutable solution here; one option would be to use a frozenset, but |
| 22 |
then one has to make classes to put into that (which are still |
| 23 |
mutable). That is a misuse for what could just be a dictionary. |
| 24 |
|
| 25 |
> > move it into the class definition. |
| 26 |
> > > def getNonSystemArchiveDepends(fetchlist, eapi): |
| 27 |
> > > ... |
| 28 |
> > > |
| 29 |
> > > ARCHIVERS = { |
| 30 |
> > > ... |
| 31 |
> > > } |
| 32 |
> > |
| 33 |
> > That makes it a non-static function variable? This is a regression. |
| 34 |
> |
| 35 |
> I guess I am not seeing why it must be a static function variable. |
| 36 |
> Can you explain? |
| 37 |
|
| 38 |
Because you would call re.compile for each time that function is |
| 39 |
called; while the most recent compiled versions of re.compile are |
| 40 |
cached, I still do not see a reason for this variable not to be static. |
| 41 |
|
| 42 |
> For the colon's in dicts thing: |
| 43 |
> |
| 44 |
> http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Whitespace |
| 45 |
|
| 46 |
Okay, I will follow those guidelines. |
| 47 |
|
| 48 |
> The @system set in gentoo will ensure these are installed. |
| 49 |
|
| 50 |
You can compare @system against the PMS and you will note that entries |
| 51 |
are missing in @system; the @system set only covers the most popular |
| 52 |
ones, the rest is left up to the maintainer to add to the ebuild. Thus |
| 53 |
this enumerates all of them; as the @system set can change in the |
| 54 |
future, we need to make the code future proof hence the @system check. |
| 55 |
|
| 56 |
It is possible for such atom to get removed from @system later. |
| 57 |
|
| 58 |
> understand the wording of PMS (as the dependencies should be |
| 59 |
> expressed somewhere) but in general we prefer to do that in @system. |
| 60 |
> For the same reason all packages do not depend on glibc, or the |
| 61 |
> compiler, or anything else. |
| 62 |
|
| 63 |
Things that are in @system are not complained about by this code: |
| 64 |
|
| 65 |
if format not in system_set_atoms: |
| 66 |
|
| 67 |
-- |
| 68 |
With kind regards, |
| 69 |
|
| 70 |
Tom Wijsman (TomWij) |
| 71 |
Gentoo Developer |
| 72 |
|
| 73 |
E-mail address : TomWij@g.o |
| 74 |
GPG Public Key : 6D34E57D |
| 75 |
GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D |
Archives