1 |
On 06/09/2016 11:54 PM, Jason Zaman wrote: |
2 |
> On Thu, Jun 09, 2016 at 08:19:43AM -0400, NP-Hardass wrote: |
3 |
>> # Old EAPIs are banned. |
4 |
>> case "${EAPI:-0}" in |
5 |
>> 5|6) ;; |
6 |
>> *) die "EAPI=${EAPI} is not supported" ;; |
7 |
>> esac |
8 |
> |
9 |
> How reasonable would it be to ban EAPI5 as well? This is a new eclass so |
10 |
> it may be better to just take the time to go to EAPI6 directly and not |
11 |
> have to migrate 5->6 later on. |
12 |
> |
13 |
> |
14 |
Would be quite reasonable. I thought since all the existing MATE |
15 |
ebuilds are EAPI 5 or 6, it'd be wise to support both, but I've only |
16 |
actually used (and probably will only use) the eclass with EAPI6 ebuilds. |
17 |
>> # @FUNCTION: python_cond_func_wrap |
18 |
>> # @DESCRIPTION: Wraps a function for conditional python use, to run for each |
19 |
>> # python implementation in the build directory. |
20 |
>> python_cond_func_wrap() { |
21 |
>> if use python; then |
22 |
>> python_foreach_impl run_in_build_dir "$@" |
23 |
>> else |
24 |
>> $@ |
25 |
>> fi |
26 |
>> } |
27 |
> |
28 |
> I dont see where you inherited the python eclasses? You also probably |
29 |
> need to use use_if_iuse (from eutils.eclass) instead since it seems like |
30 |
> python might not be in all the ebuilds. Afaik, you're not allowed to |
31 |
> call use foo if foo is not in IUSE already |
32 |
I forgot to include a comment in the ebuild, but the intention was that |
33 |
the eclass would not inherit python, by design, those wanting to use |
34 |
that should inherit python themselves. Although, if I should check that |
35 |
explicitly, I am unaware of how to do so, and would appreciate advice |
36 |
from someone, if it is possible. I am aware of INHERITED, but the PMS |
37 |
says it shouldn't be exported to ebuilds, so I'm unsure if/how it could |
38 |
be used. |
39 |
|
40 |
My hope was that since this is used several times (though, not too |
41 |
many), that I could move this logic into the eclass, but if it would be |
42 |
more appropriate to just keep that in each of those ebuilds, I can do |
43 |
that too. |
44 |
> |
45 |
> Also I'm not sure if having functions start with python_ could lead to |
46 |
> conflicts later on with the python eclasses? |
47 |
Yeah, I should probably adjust the namespace |
48 |
> |
49 |
> Other than these minor things, look good! |
50 |
> -- Jason |
51 |
> |
52 |
> |
53 |
Thanks for taking the time to look it over and review. |
54 |
|
55 |
-- |
56 |
NP-Hardass |