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

make Collection subclassable #342

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Conversation

lsaffre
Copy link
Contributor

@lsaffre lsaffre commented Mar 25, 2016

I would like to extend the invoke.Collection class by subclassing it. But the from_module method in the current version will always return the original class. This little patch has no influence on the API, is cleaner, and would make me happy :-)

@codecov-io
Copy link

Current coverage is 92.78%

Merging #342 into master will decrease coverage by -0.67% as of 566c109

@@            master    #342   diff @@
======================================
  Files           20      20       
  Stmts         1773    1773       
  Branches       305     304     -1
  Methods          0       0       
======================================
- Hit           1657    1645    -12
+ Partial         34      33     -1
- Missed          82      95    +13

Review entire Coverage Diff as of 566c109

Powered by Codecov. Updated on successful CI builds.

@bitprophet
Copy link
Member

Thanks, that's an oversight/bug for sure, adding to milestone :)

@bitprophet
Copy link
Member

Years later...sob...merged this & made sure it played nice with how the code changed in the interim. (tl;dr we moved things around & are using an inner closure, but were still hardcoding Collection instead of cls.)

@bitprophet bitprophet merged commit 75c71cd into pyinvoke:master Jan 29, 2018
bitprophet added a commit that referenced this pull request Jan 29, 2018
bitprophet added a commit that referenced this pull request Jan 29, 2018
@lsaffre
Copy link
Contributor Author

lsaffre commented Feb 15, 2018

Great! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants