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

refactor: replace imp with importlib #919

Closed
wants to merge 8 commits into from

Conversation

kuwv
Copy link
Contributor

@kuwv kuwv commented Feb 20, 2023

@bitprophet This is mostly there. Just need to sort the issues with program.

@kuwv
Copy link
Contributor Author

kuwv commented Feb 23, 2023

@bitprophet looks like I got the tests working. Need to remove load_module and this should be ready to go. Can you confirm if this looks fine or not?

@kuwv
Copy link
Contributor Author

kuwv commented Feb 23, 2023

@bitprophet alright importlib is passing. Ready for review!

@kuwv kuwv changed the title refactor: deprecate imp for importlib refactor: replace imp with importlib Feb 23, 2023
@kuwv
Copy link
Contributor Author

kuwv commented Feb 23, 2023

#675
#829

@kuwv
Copy link
Contributor Author

kuwv commented Feb 24, 2023

I have additional changes to remove the parent path from 'load()' and retrieve it from the module. But that's something that could have been done before so...

@joerg-tt
Copy link

Great! Hope this gets merged soon.

@kuwv
Copy link
Contributor Author

kuwv commented Mar 1, 2023

@jefftriplett could you give this a review?

@kuwv
Copy link
Contributor Author

kuwv commented Mar 7, 2023

@bitprophet ping

@bitprophet
Copy link
Member

I'll try to find the time to review this in depth next time I'm doing Invoke-y things. Pairs well with the typing update that is yet unreleased, I'd prob pop both out as a 'support' minor version bump when I do this. thanks! 🙏🏻

@kuwv
Copy link
Contributor Author

kuwv commented Mar 10, 2023

Great to see you back.

For the release... there's one caveat, I removed decorator.py because it wasn't used but then realized it's being consumed by fabric.

@bitprophet
Copy link
Member

Thrusting my head above the surface of the Paramiko swamp for a gasp of air & to actually look at this 🙏🏻 unless something drastic happens I'll try to pop out invoke 2.1 today with this plus all of your other recent work!

@bitprophet
Copy link
Member

Rebased onto an unrelated fix so now github's behind the times, but this is merged and on pypi! thanks again 🙏🏻

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.

3 participants