Omitir directorio .overrides/logo para ln y mejoras #1840
Conversation
- Se omite el directorio logo, para que `ln` no genere warnings. - Se cambian algunas cosas en el archivo `conf.py` para solucionar algunos warnings de flake8
| @@ -61,14 +68,18 @@ | |||
| extensions = _extensions | |||
|
|
|||
|
|
|||
| if not os.environ.get('SPHINX_GETTEXT') == 'True': | |||
| if os.environ.get('SPHINX_GETTEXT') is None: | |||
There was a problem hiding this comment.
Me imagino que está explícito, por si tiene un valor, cual sea.
| if os.environ.get('SPHINX_GETTEXT') is None: | |
| if not os.environ.get('SPHINX_GETTEXT'): |
There was a problem hiding this comment.
Era para hacerlo explícito la verdad, a pesar que tenemos cosas tan raras como:
>>> not None
Trueera para dejar en claro que quería mirar si get retornaba None, explicando así que la variable no está definida.
El problema con usar not os.environ.get(...) es que si la variable está definida a nada, entrará, y la idea es que si alguien ya la definió por alguna razón, que no entre. Por ejemplo:
>>> import os
>>> r = os.environ.get("LALA")
>>> r
>>> r is None
True
>>>
% export LALA=""
% export LELE=0
% python
>>> import os
>>> os.environ.get("LALA")
''
>>> os.environ.get("LELE")
'0'
>>> not os.environ.get("LALA")
True
>>> not os.environ.get("LELE")
False
>>> not os.environ.get("ALGO_QUE_NO_EXISTE")
Truepero para términos de este script, dudo que en algún momento alguien defina la variable a algo más :P pero es la costumbre de estar preparado para todo jaja
There was a problem hiding this comment.
Está bien chistoso porque justo el 0 arroja un False jajaja.
Igual pues, con que te regrese un '' o no esté (None) daría True, pero sí, igual y un poco exagerado, el is None está bastante explícito, bueno saberlo la verdad.
| # Override all the files from ``.overrides`` directory | ||
| from pathlib import Path | ||
| overrides_paths = Path('.overrides') | ||
|
|
||
| for path in overrides_paths.glob('**/*.*'): | ||
| if path.name == 'README.rst' and path.parent == '.overrides': | ||
| continue | ||
| # Skip the files in the .overrides/logo directory | ||
| # to avoid ln issues. | ||
| if str(path.parent).endswith("logo"): |
There was a problem hiding this comment.
Creen que valga la pena colocar este if y el anterior como filtros, y sacar la acción del ln -nfs a una función separada o es demasiado?
There was a problem hiding this comment.
como estaban motivadas por cosas distintas, preferí dejarlo separado, mal que mal, lo del logo es bastante nuevo.
Pensé en dejarlo ahí mismo en el if anterior pero después nos quedaba una condición muy fea la verdad.
No sé si ganemos muchos con el tema de la función, total el archivo conf.py tiene como objetivo ser un archivo de configuración y si miras no hay funciones para nada aparte de setup que es la única función que nos importa ahí.
|
Uso el review de Erick para hacer merge 🎉 |
lnno genere warnings.conf.pypara solucionar algunos warnings de flake8