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

compatibility import cleanup #906

Merged
merged 5 commits into from Jun 9, 2017

Conversation

Projects
None yet
3 participants
@ploxiln
Contributor

ploxiln commented Feb 23, 2017

Remove try/except blocks which were for supporting old versions of python which paramiko no longer supports.

cStringIO was actually used unconditionally before the python3 port: 0b7d0cf

@coveralls

This comment has been minimized.

coveralls commented Feb 23, 2017

Coverage Status

Coverage increased (+0.1%) to 74.326% when pulling aa8163c on ploxiln:import_cleanup into 421f03b on paramiko:master.

@ploxiln

This comment has been minimized.

Contributor

ploxiln commented Feb 23, 2017

Side note: it's a bit odd that the latest version of Fabric supports python2.5, while no supported version of paramiko does. It looks like the last version of paramiko that supported python2.5 was 1.12.4 from May of 2014.

@coveralls

This comment has been minimized.

coveralls commented Feb 23, 2017

Coverage Status

Coverage increased (+0.2%) to 74.367% when pulling 6a51506 on ploxiln:import_cleanup into 421f03b on paramiko:master.

@ploxiln

This comment has been minimized.

Contributor

ploxiln commented Feb 23, 2017

added in a bit more general import cleanup

@bitprophet

This comment has been minimized.

Member

bitprophet commented Feb 23, 2017

Awesome! Looks good overall. Can you rebase against 1.18 and also open a separate PR against 2.0? For the short term I don't want to leave 1.x folks completely hanging so I'm trying to backport most changes there. (But since it requires carefully cherry-picking to 2.0, so 2.0 doesn't get feature work from 1.18, I'm probably going to stop soon. Or maybe just stop updating 2.0...hrm.)

The Fabric thing is largely laziness on my part / the fact that 2.0 is looming. Paramiko got a Py3 compat overhaul in 1.13 which is when it dropped 2.5 support; Fabric 1 can still arguably only work on Python 2 so a user with Paramiko 1.12 and Fabric 1 and Python 2.5 should still be able to operate.

@ploxiln ploxiln changed the base branch from master to 1.18 Feb 23, 2017

@ploxiln

This comment has been minimized.

Contributor

ploxiln commented Feb 23, 2017

rebased on 1.18
(since this is not really features nor bug fixes, wasn't sure it made sense to do so)

I think it's not worth the effort and confusing git history to up-merge from 1.18 to 2.0, you'll probably want to start doing 1.17/1.18 together and 2.0/2.1/+ together (cherry-picking back to the older "bundle" of branches). Or just cherry-pick back for each, that's not uncommon.

@coveralls

This comment has been minimized.

coveralls commented Feb 23, 2017

Coverage Status

Coverage increased (+0.1%) to 74.493% when pulling 964dd6c on ploxiln:import_cleanup into cb4fc8c on paramiko:1.18.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Feb 23, 2017

Yes, that's exactly what I've been doing so far, ever since 1.18/2.1 came out:

  • get the code into 1.17 via rebase or cherry-pick (this is now no longer necessary, I figured having a large support window was simply not worth the extra overhead)
  • merge up into 1.18
  • get the code into 2.0 via cherry-pick
  • merge up into 2.1 and master

It's just that, it's kind of a pain in the rear ;) especially since I had such a strong habit of simply "merge up up up!" from the past.

Anyway, I'll do the needful based on this, thank you!

@bitprophet bitprophet added the Support label Feb 23, 2017

@bitprophet bitprophet added this to the 1.18.3 etc milestone Feb 23, 2017

bitprophet added a commit that referenced this pull request Jun 9, 2017

@bitprophet bitprophet merged commit 964dd6c into paramiko:1.18 Jun 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

bitprophet added a commit that referenced this pull request Jun 9, 2017

@ploxiln ploxiln deleted the ploxiln:import_cleanup branch Sep 13, 2017

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