Skip to content

[4.6.x] Fix one issue for compat::getfuncargnames #6785

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

Closed

Conversation

SeanXu1984
Copy link

Fix so the self of an instancemethod will not be removed again in getfuncargnames because it is already removed from signature(function)

I will deal with changelog later.

…funcargnames because it is already removed from signature(function)
…funcargnames because it is already removed from signature(function)
…funcargnames because it is already removed from signature(function)
@SeanXu1984
Copy link
Author

@nicoddemus I don't think the CI failure was introduced by my change. Could you help me with this please?

@RonnyPfannschmidt
Copy link
Member

Travis seems to fail with tmpdir cleanup

…funcargnames because it is already removed from signature(function)
@bluetech bluetech changed the title Fix one issue for compat::getfuncargnames [4.6.x] Fix one issue for compat::getfuncargnames Apr 10, 2020
@bluetech
Copy link
Member

I've edited the title to add [4.6.x] since it targets that branch.

@Zac-HD
Copy link
Member

Zac-HD commented Jun 2, 2020

@SeanXu1984 - it looks like this issue also affects the master branch. Could you please

  1. Describe the problem that this patch fixes? Ideally by adding a test that failed before and passes now, but a prose description would also be OK.
  2. Open a pull request targeting the master branch instead; we can backport it to 4.6 after merging.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

as far as i can tell this one tries to add bound methods, which is not something pytest itself collects

we need a accompanying unit-test at least and a reference for the usage, else its bound to cause confusion later on

also in order to be integrated properly we need it as feature in master and do a back-port later on

@SeanXu1984
Copy link
Author

I've created another PR for main branch. #6882

@nicoddemus
Copy link
Member

Closing for now as in #6882. 👍

@nicoddemus nicoddemus closed this Jun 13, 2020
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.

5 participants