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

Require \ for asterisks in Sphinx-style parameter docstrings #5464

Merged
merged 5 commits into from Dec 3, 2021

Conversation

DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Type of Changes

Type
🐛 Bug fix

Description

Closes #5406

This took way longer than it should have.. Never do regex on a Friday..

@DanielNoord DanielNoord added Regression False Positive 🦟 A message is emitted but nothing is wrong with the code labels Dec 3, 2021
@DanielNoord DanielNoord added this to the 2.12.2 milestone Dec 3, 2021
@coveralls
Copy link

coveralls commented Dec 3, 2021

Pull Request Test Coverage Report for Build 1535712912

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0004%) to 93.532%

Totals Coverage Status
Change from base Build 1535710960: 0.0004%
Covered Lines: 14026
Relevant Lines: 14996

💛 - Coveralls

@Pierre-Sassoulas
Copy link
Member

Never do regex on a Friday..

s/' on a Friday.'/''/g 😄

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

I was joking about regex, I like them fondly. But the approach with regex might have been an original design mistake. Good job managing with that though. The fix is quite dandy, Also the more tests we have the more manageable a migration from regex in the future will be if hard to fix bugs keep piling up. Chances are most case are handled and we won't have to change this too often.

@DanielNoord
Copy link
Collaborator Author

I was joking about regex, I like them fondly. But the approach with regex might have been an original design mistake. Good job managing with that though. The fix is quite dandy, Also the more tests we have the more manageable a migration from regex in the future will be if hard to fix bugs keep piling up. Chances are most case are handled and we won't have to change this too often.

I do to, but what is really annoying is how these patterns are written. It doesn't allow you to easily copy them over to regex101.com or a similar engine. That has really frustrated my process of working on these..

DanielNoord and others added 2 commits December 3, 2021 15:25
@DanielNoord DanielNoord merged commit 608ed32 into pylint-dev:main Dec 3, 2021
@DanielNoord DanielNoord deleted the sphinx-args branch December 3, 2021 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v2.12 Regression] Asterisk argument mising-param-doc vs anomalous-backslash-in-string
4 participants