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 51973) partially unify public functions signature for pkg and lowpkg #54999

Closed
wants to merge 2 commits into from

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Oct 14, 2019

What does this PR do?

The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.

Also do the same for 'lowpkg'

Tests written?

No, no change in behavior. Current tests are covering the refactoring.

(backport #51973, already merged in develop)

The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

  TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.

(cherry picked from commit fbb8a29)
In the same way that the previous commit for pkg, add **kwargs
into the public function signature for lowpkg virtual module, to
unify the different signatures.

(cherry picked from commit c9473a0)
@aplanas
Copy link
Contributor Author

aplanas commented Oct 14, 2019

After a second I think that is better to close this and add it into #54954

@aplanas aplanas closed this Oct 14, 2019
@aplanas aplanas deleted the backport_51973 branch April 7, 2020 16:06
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.

1 participant