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

disable linting in test folders #776

Merged
merged 6 commits into from
Jul 1, 2020
Merged

disable linting in test folders #776

merged 6 commits into from
Jul 1, 2020

Conversation

ZacLN
Copy link
Contributor

@ZacLN ZacLN commented Jun 22, 2020

No description provided.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
mmarchini mary marchini
@ZacLN ZacLN added this to the Next extension patch release milestone Jun 22, 2020
@ZacLN ZacLN self-assigned this Jun 22, 2020
src/utilities.jl Outdated
Comment on lines 337 to 339
if VERSION < v"1.1"
return false
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you just forgot to remove this check ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

@pfitzseb
Copy link
Member

Does this disable all linting or only "global" linting? It would be nice to still have some for of checking in local files, for example.

@ZacLN
Copy link
Contributor Author

ZacLN commented Jun 24, 2020

I don't understand what you mean. This disables linting for files located within a "test" sub-directory of a folder that is a Julia package folder if that makes it clearer?

@pfitzseb
Copy link
Member

Right, but it would be cool if it didn't disable all linting -- some features are local to the current file (and maybe includes), but don't need to care about packages at all. E.g. the analysis of which function arguments are used etc. So if possible we should keep that kind of stuff enabled.

@davidanthoff
Copy link
Member

@ZacLN and @pfitzseb what do we do about this as a next step?

@aviatesk
Copy link
Member

Is it easy to do "basic lintings" (i.e. syntax-errors and such, those could be done by per-expression or per-file) separately from "more complex linting" (like warning on undef name, which would be done by per-file or per-package level) ?
If not, I think we want to mergd this as is and do the separation first in another PR ?

@ZacLN
Copy link
Contributor Author

ZacLN commented Jul 1, 2020

This now disables lint checks that are dependent on having an accurate representation of the package environment. Not a particularly elegant approach, I need to start categorising LintCodes.

@pfitzseb
Copy link
Member

pfitzseb commented Jul 1, 2020

Nice, this is exactly what I had in mind! Can always fix up the implementation at a later point

@davidanthoff davidanthoff merged commit d9c4678 into master Jul 1, 2020
@davidanthoff davidanthoff deleted the disable-test-lint branch July 1, 2020 19:13
@oppo-source oppo-source removed the request for review from a team April 16, 2021 07:35
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 this pull request may close these issues.

None yet

4 participants