Fixes to support Python 2.6 again.#541
Uh oh!
There was an error while loading. Please reload this page.
Conversation
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
|
andy-maierforce-pushed the py26_fixes branch 4 times, most recently from 7e35290 to a795f40Compare
Byron commented Oct 22, 2016
@andy-maier In the meanwhile, merge conflicts have shown up. I will gladly merge this PR once they are fixed. Thank you. |
andy-maier commented Oct 23, 2016
Will do, tomorrow. |
andy-maierforce-pushed the py26_fixes branch 7 times, most recently from 9364ea0 to 9ff3f02Compare
Details: - Added Python 2.6 again to .travis.yml (it was removed in commit 4486bcb). - Replaced the use of dictionary comprehensions in `git/cmd.py` around line 800 with the code before that change (in commit 25a2ebf). 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).
andy-maier commented Oct 24, 2016
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. |
ankostis commented Oct 24, 2016
@andy-maier have you taken into account my 2 review comments? |
andy-maier commented Oct 24, 2016
No, I'm sorry. I was not aware of your comments. Where can I find them? |
ankostis commented Oct 24, 2016
They are visibile in this PR, a couple of comments above embedded. |
andy-maier commented Oct 25, 2016
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? |
ankostis commented Oct 25, 2016
Never mind, i will push on top some commits. |
ankostis commented Oct 29, 2016
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. |
Byron commented Dec 8, 2016
@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. |
| # otherwise these'll end up in args, which is bad. | ||
| _kwargs={k: vfork, vinkwargs.items() ifkinexecute_kwargs} | ||
| kwargs={k: vfork, vinkwargs.items() ifknotinexecute_kwargs} | ||
| _kwargs=dict() |
There was a problem hiding this comment.
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)| ddt | ||
| mock | ||
| mock | ||
| unittest2; python_version<'2.7' |
There was a problem hiding this comment.
Can you modify also setup.py and test-requirements.txt?
ifsys.version_info[:2] < (2, 6): test_requires.append('unittest2')
Apply codereview comments of gitpython-developers#541 (review)
Apply codereview comments of gitpython-developers#541 (review)
Apply codereview comments of gitpython-developers#541 (review)
Apply codereview comments of gitpython-developers#541.
ankostis commented Dec 8, 2016
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. |
4 participants
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.