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

Improve docs clarity on loading custom checkers - FIXED #7502

Merged
merged 10 commits into from
Sep 20, 2022
Merged

Improve docs clarity on loading custom checkers - FIXED #7502

merged 10 commits into from
Sep 20, 2022

Conversation

daogilvie
Copy link
Contributor

NOTE: This PR replaces #7276 due to an admin error on my part.

Type of Changes

Type
βœ“ πŸ“œ Docs

Description

Short

This PR just updates the documentation page on writing custom checkers. It adds
information about some restrictions between how init-hook changes the loading
path and when you can use that to import your custom module.

Long (in the commit message)

Whilst the docs on testing custom checkers do touch on the fact the
module has to be in your load path, there are some restrictions that
were not made clear before.
Specifically, the init-hook configuration item (which is often used
explicitly to change the sys.path) is not automatically sufficient. If
the init hook is in a file, but the load-plugins argument is passed in
the CLI, then the plugins are loaded before the init-hook has run, and
so will not be imported. In this case, the failure can also be silent,
so the outcome is a lint that will simply not contain any information
from the checker, as opposed to a run that will error.
This corner case may merit fixing in a separate PR, that is to be
confirmed.

Closes #7264

daogilvie and others added 9 commits August 8, 2022 15:03
Whilst the docs on testing custom checkers do touch on the fact the
module has to be in your load path, there are some restrictions that
were not made clear before.
Specifically, the init-hook configuration item (which is often used
explicitly to change the sys.path) is not automatically sufficient. If
the init hook is in a file, but the load-plugins argument is passed in
the CLI, then the plugins are loaded before the init-hook has run, and
so will not be imported. In this case, the failure can also be silent,
so the outcome is a lint that will simply not contain any information
from the checker, as opposed to a run that will error.
This corner case may merit fixing in a separate PR, that is to be
confirmed.

Closes #7264
As per suggestion from @DanielNoord, this makes it clearer that
installing the custom module is the recommended way.
Thanks to @DanielNoord

Co-authored-by: DaniΓ«l van Noord <13665637+DanielNoord@users.noreply.github.com>
This is not the way.

Co-authored-by: DaniΓ«l van Noord <13665637+DanielNoord@users.noreply.github.com>
Co-authored-by: DaniΓ«l van Noord <13665637+DanielNoord@users.noreply.github.com>
This is a much clearer way of explaining what is meant by this recommendation.

Co-authored-by: DaniΓ«l van Noord <13665637+DanielNoord@users.noreply.github.com>
Thanks to Pierre Sassoulas and Daniel Noord
@coveralls
Copy link

coveralls commented Sep 20, 2022

Pull Request Test Coverage Report for Build 3088428075

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 95.326%

Totals Coverage Status
Change from base Build 3087316542: 0.0%
Covered Lines: 17069
Relevant Lines: 17906

πŸ’› - Coveralls

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Final points!

doc/development_guide/how_tos/custom_checkers.rst Outdated Show resolved Hide resolved
doc/development_guide/how_tos/custom_checkers.rst Outdated Show resolved Hide resolved
@@ -0,0 +1,3 @@
Dev documentation is now clearer about custom checker loading.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed.

Comments thanks to Daniel Noord
@DanielNoord DanielNoord added Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry labels Sep 20, 2022
@DanielNoord
Copy link
Collaborator

Thank you very much for this and all other work related to it @daogilvie πŸ˜„

@DanielNoord DanielNoord merged commit e1896ed into pylint-dev:main Sep 20, 2022
@daogilvie daogilvie deleted the issue-7264-clearer-doc-on-plugin-sourcing branch September 20, 2022 08:11
@daogilvie
Copy link
Contributor Author

You are very welcome, thanks for improving and maintaining a project that has saved me from myself more times than I can remember πŸ˜‚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot get pylint to register custom checker, is there a missing step in the docs?
3 participants