1 |
On Wed, Nov 30, 2016 at 3:38 AM, konsolebox <konsolebox@×××××.com> wrote: |
2 |
> There are some things I noticed in the tmpfiles_process() function: |
3 |
> |
4 |
> - `type` currently also checks for functions, alias, and builtins, |
5 |
> besides executable files. If that's not intended, the `-P` option |
6 |
> should be added. |
7 |
|
8 |
I cannot conceive of any way a function or alias would be defined with |
9 |
these names unless an ebuild author did it intentionally or a user did |
10 |
it via /etc/portage/bashrc. In either case, I see no need to prevent |
11 |
that. |
12 |
|
13 |
> - `[[ ${ROOT} == / ]] || return 0` seems to present a harmless false |
14 |
> condition, and it doesn't show an error message. I would be helpful |
15 |
> to have a comment added above it to give details why. |
16 |
|
17 |
We only want to process tmpfiles for the currently running system. |
18 |
|
19 |
If ROOT is not /, it indicates we are installing the package for use |
20 |
on a different system or in a container. In either case, the tmpfiles |
21 |
would be processed upon boot when that system/container is started. |
22 |
|
23 |
I find this fairly obvious, but if William wants to document it that's fine. |
24 |
|
25 |
> I also prefer some things this way: |
26 |
> |
27 |
> - Indent the contents of the first `if` block for consistency's sake, |
28 |
> and less confusion. |
29 |
|
30 |
I disagree; indenting the entire eclass is silly and does not really |
31 |
improve readability. Also, this is a very common pattern found in |
32 |
other eclasses. |
33 |
|
34 |
> - Patterns in the `case` block doesn't have to be indented. This makes |
35 |
> the contents of the `case` block aligned with the contents of the |
36 |
> other blocks (`if`, `while`, etc.), and it makes the use of indents at |
37 |
> minimum when the block is used recursively. |
38 |
|
39 |
Sorry, I have no idea what you're trying to say here. Recursive blocks? |
40 |
|
41 |
This really feels like you're making a personal style suggestion here, |
42 |
and I personally see nothing wrong with it as-is. |