Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix error when using is_version with lettered tmux version#22

Merged
tony merged 4 commits into
tmux-python:mastertmux-python/libtmux:masterfrom
minijackson:patch-1minijackson/libtmux:patch-1Copy head branch name to clipboard
Jan 20, 2017
Merged

Fix error when using is_version with lettered tmux version#22
tony merged 4 commits into
tmux-python:mastertmux-python/libtmux:masterfrom
minijackson:patch-1minijackson/libtmux:patch-1Copy head branch name to clipboard

Conversation

@minijackson
Copy link
Copy Markdown
Contributor

is_version('something') fails with installed tmux 1.9a

To allow tmux-python/tmuxp#197 ;-)

is_version('something') fails with installed tmux 1.9a

To allow tmux-python/tmuxp#197 ;-)
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 14, 2017

Current coverage is 78.74% (diff: 100%)

Merging #22 into master will increase coverage by 0.49%

@@             master        #22   diff @@
==========================================
  Files             8          8          
  Lines           814        814          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            637        641     +4   
+ Misses          177        173     -4   
  Partials          0          0          

Powered by Codecov. Last update cfbaabe...fb77c7c

@tony
Copy link
Copy Markdown
Member

tony commented Jan 14, 2017

@minijackson Can this have a test for it please?

@tony
Copy link
Copy Markdown
Member

tony commented Jan 14, 2017

@minijackson
Copy link
Copy Markdown
Contributor Author

There you go!

But I don't know how to mock the installed tmux to be tmux 1.9a, so while the case that the requested tmux version is "lettered" is tested, the installed tmux case is not.

@tony
Copy link
Copy Markdown
Member

tony commented Jan 14, 2017

Travis automatically tests across those tmux versions. https://github.com/tony/libtmux/blob/master/.travis.yml

also you could manually insert the version number for '1.9a'

@minijackson
Copy link
Copy Markdown
Contributor Author

also you could manually insert the version number for '1.9a'

I didn't understand, is it something I have to do for this PR ? Where should I insert the version number ?

@tony
Copy link
Copy Markdown
Member

tony commented Jan 15, 2017

That is fine.

The only thing I'm double checking is how broad support is for pkg_resources. Are there any corner cases where the user may not have setuptools and this patch could lead to a ImportError: No module named 'pkg_resources'.

@tony
Copy link
Copy Markdown
Member

tony commented Jan 15, 2017

Even though most machines are going to have setuptools on it, after further consideration it may be safer to use a method without it. I'm not sure what the ramifications would be for upstream packages for tmuxp on arch and slack.

@tony
Copy link
Copy Markdown
Member

tony commented Jan 15, 2017

Try LooseVersion instead of StrictVersion.

@tony tony merged commit f9fd3cb into tmux-python:master Jan 20, 2017
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.