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

[Backport maintenance/2.15.x] Fix deprecated-method false positive #7804

Merged

Conversation

github-actions[bot]
Copy link
Contributor

Backport 57f38c3 from #7795.

@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow Skip news 🔇 This change does not require a changelog entry labels Nov 19, 2022
@Pierre-Sassoulas
Copy link
Member

First back-port that worked, the benefit is that we don't have to do the cherry-picking manually and we will always be able to release with minimal work because the maintenance branch will be ready. But we do have to add the labels and approve the PR.

@Pierre-Sassoulas
Copy link
Member

@DanielNoord, I excluded the primer on the maintenance branch, what's your opinion about that ?

@Pierre-Sassoulas Pierre-Sassoulas added Needs review 🔍 Needs to be reviewed by one or multiple more persons and removed Blocked 🚧 Blocked by a particular issue labels Nov 20, 2022
@Pierre-Sassoulas Pierre-Sassoulas removed the Skip news 🔇 This change does not require a changelog entry label Nov 20, 2022
@DanielNoord
Copy link
Collaborator

@DanielNoord, I excluded the primer on the maintenance branch, what's your opinion about that ?

I think we should probably still run it. The cherry picking might introduce hard to spot bugs because of dependent commits we don't know about.
Seeing as it wouldn't run that often it also doesn't cost us too much CI time.

When alias for method is similar to name of deprecated method.

Closes #5886

(cherry picked from commit 57f38c3)
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the backport-7795-to-maintenance/2.15.x branch from e0a3f65 to 38d2f23 Compare November 22, 2022 22:22
@Pierre-Sassoulas
Copy link
Member

Testing the configuration added to main / maintenance/2.15.x with #7815

@Pierre-Sassoulas
Copy link
Member

@DanielNoord it seems the primer can't run on the maintenance branch right now:

 Archive:  primer_commitstring.zip
  inflating: commit_string_3.11.txt  
  inflating: commit_string_3.7.txt   
cp: cannot stat 'commit_string.txt': No such file or directory

What do you think ?

@coveralls
Copy link

coveralls commented Nov 22, 2022

Pull Request Test Coverage Report for Build 3530420845

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.0005%) to 95.34%

Totals Coverage Status
Change from base Build 3527537560: -0.0005%
Covered Lines: 17083
Relevant Lines: 17918

💛 - Coveralls

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Nov 22, 2022

I wouldn't bother running the primer on the maintenance branches. If we've merged work to main, it's far too late to be parsing and investigating if changes are false positives or intended changes on the support branches.

@Pierre-Sassoulas
Copy link
Member

Also if the primer caugth something it's going to be added to our functional tests before being merged so there's less risks.

@DanielNoord
Copy link
Collaborator

I suggested it initially as it might catch merge conflicts that are not directly exposed by the tests. However, if it is such a hassle then let's not.

@Pierre-Sassoulas Pierre-Sassoulas marked this pull request as ready for review November 23, 2022 08:18
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the backport-7795-to-maintenance/2.15.x branch from 9bc99c3 to 94223f4 Compare November 23, 2022 08:21
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the backport-7795-to-maintenance/2.15.x branch from 94223f4 to 9221484 Compare November 23, 2022 08:29
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the backport-7795-to-maintenance/2.15.x branch from 9221484 to 8e44095 Compare November 23, 2022 08:33
@Pierre-Sassoulas
Copy link
Member

I wanted to disable some more checks but it requires a refactor because on: needs to be defined for the whole file and I want to keep the pylint check in "Check". This will wait for 2.16's maintenance.

@Pierre-Sassoulas Pierre-Sassoulas merged commit dff0adb into maintenance/2.15.x Nov 23, 2022
@Pierre-Sassoulas Pierre-Sassoulas deleted the backport-7795-to-maintenance/2.15.x branch November 23, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants