Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork938
Fixes to support Python 2.6 again.#541
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecov-io commentedOct 21, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Current coverage is 94.51% (diff: 96.72%)@@ master #541 diff @@========================================== Files 63 63 Lines 9882 9920 +38 Methods 0 0 Messages 0 0 Branches 0 0 ==========================================+ Hits 9326 9376 +50+ Misses 556 544 -12 Partials 0 0
|
7e35290 toa795f40Compare@andy-maier In the meanwhile, merge conflicts have shown up. I will gladly merge this PR once they are fixed. Thank you. |
Will do, tomorrow. |
9364ea0 to9ff3f02CompareDetails:- Added Python 2.6 again to .travis.yml (it was removed in commit4486bcb).- Replaced the use of dictionary comprehensions in `git/cmd.py` around line 800 with the code before that change (in commit25a2ebf). Reason: dict comprehensions were introduced only in Python 2.7.- Changed the import source for `SkipTest` and `skipIf` from `unittest.case` to first trying `unittest` and upon ImportError from `unittest2`. This was done in `git/util.py` and in several testcases. Reason: `SkipTest` and `skipIf` were introduced to unittest only in Python 2.7, and `unittest2` is a backport of `unittest` additions to Python 2.6.- In git/test/lib/helper.py, fixed the definition of `assertRaisesRegex` to work on py26.- For Python 2.6, added the `unittest2` dependency to `requirements.txt` and changed `.travis.yml` to install `unittest2`. Because git/util.py uses SkipTest from unittest/unittest2, the dependency could not be added to `test-requirements.txt`.- Fixed an assertion in `git/test/test_index.py` to also allow a Python 2.6 specific exception message.- In `is_cygwin_git()` in `git/util.py`, replaced `check_output()` with `Popen()`. It was added in Python 2.7.- Enabled Python 2.6 for Windows: - Added Python 2.6 for MINGW in .appveyor.yml. - When defining `PROC_CREATIONFLAGS` in `git/cmd.py`, made use of certain win32 and subprocess flags that were introduced in Python 2.7, dependent on whether we run on Python 2.7 or higher. - In `AutoInterrupt.__del__()` in `git/cmd.py`, allowed for `os` not having `kill()`. `os.kill()` was added for Windows in Python 2.7 (For Linux, it existed in Python 2.6 already).
The "rebase" got a little larger because I enabled Python 2.6 also for Windows now, and I added a Python 2.6 environment for both Appveyor (MINGW) and Travis (An earlier commit removed it from Travis). Unlike earlier, Python 2.6 is now mandatory in Travis in this PR. I think this is the best way to keep it working in the future. If and when an unsurmountable issue with Python 2.6 pops up, it can quickly be removed again. Please review again. |
@andy-maier have you taken into account my 2 review comments? |
No, I'm sorry. I was not aware of your comments. Where can I find them? |
They are visibile in this PR, a couple of comments above embedded. |
That's where I looked first, but I don't see them. Maybe something behaves different because this is a PR from my fork and I force-pushed to the branch? Anyway, could you please copy them here? |
Never mind, i will push on top some commits. |
I'm really having problem working with PY26 - there are still some unofficial python-sites for this release, but are really slow or unmaintained. As I said, this deprecated platform is not only a development burden for us but a security liability for you. |
@andy-maier Thanks a lot for your contribution, it's much appreciated! I have had a look at the commit, and believe it's OK to merge under the assumption that we can remove or revert it at any time should there be any issues cropping up, or if it becomes an additional burden to maintain it. @ankostis Please feel free to revert this commit in case it is a security liability in itself. |
| @@ -812,8 +812,12 @@ def _call_process(self, method, *args, **kwargs): | |||
| :return: Same as ``execute``""" | |||
| # Handle optional arguments prior to calling transform_kwargs | |||
| # otherwise these'll end up in args, which is bad. | |||
| _kwargs = {k: v for k, v in kwargs.items() if k in execute_kwargs} | |||
| kwargs = {k: v for k, v in kwargs.items() if k not in execute_kwargs} | |||
| _kwargs = dict() | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you replace it with this more pythonic and quicker to follow (in debug-mode also) code?
_kwargs=dict(k,v)fork,vinkwargs.items()ifkinexecute_kwargs)kwargs=dict((k,v)fork,vinkwargs.items()ifknotinexecute_kwargs)
| mock | ||
| unittest2; python_version < '2.7' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can you modify alsosetup.py andtest-requirements.txt?
ifsys.version_info[:2]< (2,6):test_requires.append('unittest2')
Apply codereview comments ofgitpython-developers#541 (review)
Apply codereview comments ofgitpython-developers#541 (review)
Apply codereview comments ofgitpython-developers#541 (review)
Apply codereview comments ofgitpython-developers#541.
No prob, the PR is fine. Just did a minor pythonism included in#555 to convert a loop as a 2 lines, to facilitate debuging. |
Uh oh!
There was an error while loading.Please reload this page.
GitPython 2.0.9 has some issues on Python 2.6 (see issue#540).
This PR fixes them. For details, see the commit message.
The Travis runs now pass on all Python versions including 2.6.
Please review.