- Notifications
You must be signed in to change notification settings - Fork397
Omitir directorio .overrides/logo para ln y mejoras#1840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 | |||
| ifnotos.environ.get('SPHINX_GETTEXT')== 'True': | |||
| if os.environ.get('SPHINX_GETTEXT')is None: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Me imagino que está explícito, por si tiene un valor, cual sea.
| ifos.environ.get('SPHINX_GETTEXT')isNone: | |
| ifnotos.environ.get('SPHINX_GETTEXT'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Era para hacerlo explícito la verdad, a pesar que tenemos cosas tan raras como:
>>>notNoneTrue
era para dejar en claro que quería mirar siget retornabaNone, explicando así que la variable no está definida.
El problema con usarnot 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:
>>>importos>>>r=os.environ.get("LALA")>>>r>>>risNoneTrue>>>%exportLALA=""%exportLELE=0%python>>>importos>>>os.environ.get("LALA")''>>>os.environ.get("LELE")'0'>>>notos.environ.get("LALA")True>>>notos.environ.get("LELE")False>>>notos.environ.get("ALGO_QUE_NO_EXISTE")True
pero 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Está bien chistoso porque justo el0 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, elis 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Creen que valga la pena colocar este if y el anterior como filtros, y sacar la acción delln -nfs a una función separada o es demasiado?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 archivoconf.py tiene como objetivo ser un archivo de configuración y si miras no hay funciones para nada aparte desetup 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