Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[INFRA] LGTM alert #864

Closed
DimitriPapadopoulos opened this issue Aug 27, 2021 · 0 comments · Fixed by #865
Closed

[INFRA] LGTM alert #864

DimitriPapadopoulos opened this issue Aug 27, 2021 · 0 comments · Fixed by #865

Comments

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos commented Aug 27, 2021

I tried to work around an LGTM alert with bb6065a in #853. Unfortunately, this doesn't work:
https://lgtm.com/projects/g/bids-standard/bids-specification/snapshot/56824aa6b869bcbec1fb14c50e3771cfc0358af5/files/pdf_build_src/process_markdowns.py?sort=name&dir=ASC&mode=heatmap#xea83c650d9db5491:1

I believe LGTM wants a lone noqa comment. If so, we need to change:

from schemacode import macros # noqa (used in "eval" call later on)

into:

# used in "eval" call later on
from schemacode import macros  # noqa

or, although I know you don't like it::

# used in "eval" call later on
from schemacode import macros  # noqa: F417 lgtm [py/unused-import]

Last option, we could even revert to:

# used in "eval" call later on
from schemacode import macros  # noqa: F417

and disable the relevant LGTM alert globally using an lgtm.yml configuration file.

Which of these three options do you prefer? I prefer option 3, as LGTM is raising a false positive that cannot be worked around without sacrificing code quality (see github/codeql#6517).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant