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

Better message documentation: add various templates to be able to handle all messages #7897

Closed
10 of 14 tasks
Pierre-Sassoulas opened this issue Dec 4, 2022 · 15 comments
Closed
10 of 14 tasks
Labels
Documentation 📗 Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Dec 4, 2022

Current problem

In #5953, some messages do not follow the generic template with good code and bad code or can't be triggered easily. Among them:

internal errors

  • astroid-error

configuration errors

  • b/bad-plugin-value

Code that require multiple examples

There already are multiple example mixed in the existing files.

Messages that depend on multiple file or dependency to trigger

other

  • too-many-lines (Hard and meaningless to do parity behavior between bad/good, creating a new file is often the solution)
  • b/bad-file-encoding (Need to be handled by the generating part)
  • c/c-extension-no-member (This one is going to be hard to trigger, not for beginner)
  • r/relative-beyond-top-level

Desired solution

We need to create templates for those message so we can generate the proper skeleton for them.

  • For internal errors, probably only a message stating "You should never encounter this one, please open an issue if you do"
  • For configuration errors a bad.toml and good.toml with some tests close to what we do for good.py / bad.py (maybe running on a dummy empty file)
  • For code that require multiple examples, handle multiple example more gracefully than right now (we just add a longer example with multiple part)

Additional context

The Sphinx extension that generates these pages needs to be modified see #5953 (comment)

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Documentation 📗 Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Dec 4, 2022
@Pierre-Sassoulas Pierre-Sassoulas changed the title Better message documentation: template for internal messages Better message documentation: add various templates to be able to handle all messages Dec 4, 2022
@ollie-iterators
Copy link
Contributor

I think it would be beneficial if the errors that need to have a different template with good.py and bad.py were moved into this or a separate PR so that people could focus on the issues that can currently have documentation added.

There could also be an explanation of what needs to be added if different things need to be added for different errors.

@ollie-iterators
Copy link
Contributor

An alternative suggestion is to add notes to each issue that needs to have a different template on #5953.

@Pierre-Sassoulas
Copy link
Member Author

add notes to each issue that needs to have a different template on #5953.

That's what I'm trying to do in the tasklist of the description in the first comment, check it out :)

@ollie-iterators
Copy link
Contributor

ollie-iterators commented Feb 12, 2023

Do you still think that unexpected-line-ending-format should be included in this list? Because it has not had a comment for that

@Pierre-Sassoulas
Copy link
Member Author

It's done in #8255

@Pierre-Sassoulas
Copy link
Member Author

It's possible to do some messages without a new template by removing good.py and bad.py and simply adding a details.rst and related.rst.

@ollie-iterators
Copy link
Contributor

import-error and import-self both have documentation

@ollie-iterators
Copy link
Contributor

I noticed that the error: https://bugs.python.org/issue20485 that is mentioned in the documentation for non-ascii-file-name has been fixed.

@Pierre-Sassoulas
Copy link
Member Author

Nice catch, it look like its been fixed for python > 3.4 (python/cpython#64684 (comment)).

@DanielNoord
Copy link
Collaborator

We still need support for configuration file messages, but with #8287 we now have support for multi-file messages 😄

@ollie-iterators
Copy link
Contributor

I think it would be beneficial if the errors that need to have a different template with good.py and bad.py were moved into this or a separate PR so that people could focus on the issues that can currently have documentation added.

There could also be an explanation of what needs to be added if different things need to be added for different errors.

I see that the errors that needed to have a different template were moved here.

@ollie-iterators
Copy link
Contributor

ollie-iterators commented Feb 21, 2023

I'm trying to use the duplicate-code example as a way to do too-many-lines (Don't worry @DanielNoord, I mentioned you in the commit), and it keeps saying that there is an "AssertionError: There should be at least one warning raised for 'bad' examples." I have max-module-lines at 15, which should cause an issue with orange.py and not for any other file but, as I said before, pytest still says that "there should be at least one warning raised ..."

NOTE: I just changed orange.py to bad.py and the code was able to find the too-many-lines error. This could be an issue with how the new template code was made.

@ollie-iterators
Copy link
Contributor

Also, how would one go about removing non-ascii-file-name from pylint?

@Pierre-Sassoulas
Copy link
Member Author

The too-many-lines examples could work without a bad/good example imo.

For non-ascii-file-name, we can either remove it or add a py-version guard. I would be in favor of removing as python 3.4 is long gone: for that there is a DELETED_MESSAGES constant to upgrade and everything related to the check should be removed (related: #6670).

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 2, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 2, 2023
DanielNoord added a commit that referenced this issue Sep 4, 2023
…rror (#8992)

* [bad-plugin-value] Initial documentation

Refs #7897

* [unrecognized-option] Initial documentation

Refs #7897

* [raw-checker-failed] Create the initial documentation

Co-authored-by: Daniël van Noord <[email protected]>
@Pierre-Sassoulas
Copy link
Member Author

Closing as completed as the template now exists and we can move and deal with the remaining messages in #5953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 📗 Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

3 participants