[READY] Adding new python-future dependency

[Valloric]

We'll use this for Python 3 compatibility. Also changed responses.py
using futurize so that it uses a module from python-future to ensure the
sys.path module pickup is working.

[Valloric]

Grr, another flaky failure. This time with omnisharp returning a 500.

[micbou]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/responses.py, line 21 [r1] (raw file):
I don't understand why this is needed. Only the os module is imported. There is no division, no print, and the strings does not contain unicode characters.

---

ycmd/responses.py, line 22 [r1] (raw file):
We are only using the str and object builtins so we should explicitly import them.

---

Comments from the review on Reviewable.io

[Valloric]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/responses.py, line 21 [r1] (raw file):
Ha, I knew this will be controversial. :)

I was thinking we might want to always have the __future__ imports up top along with all the builtins imported as well. This way we can just always write Python 3 without concern.

I wouldn't do this for anything but the Python 3 compatibility layer.

I'm worried that if we only import the parts of the shim layer that are used in the file, someone's going to change something in it which would work on one version of Python but break on another, and if we're lucky Travis would catch it. It's one thing to remember "I have to import the os module to use os.foo" and quite another to realize one must import division from __future__ before dividing any numbers. And if you forget, it's going to work on one version of Python, the one you're developing on.

So I think the shim layer might be a special case; what do the rest of you think? We need to decide how we're going to be doing this codebase-wide.

@puremourning @vheon

---

Comments from the review on Reviewable.io

[vheon]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/responses.py, line 21 [r1] (raw file):
I was thinking about this today when I read the page for writing Python 3 from scratch using python-future. I see two option:

• put this on top of the file that needs Python 3 compatibility
• use only the needed imports and put in place pylint to note when imports are needed.

---

ycmd/responses.py, line 22 [r1] (raw file):
is the same as above

---

Comments from the review on Reviewable.io

[Valloric]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/responses.py, line 21 [r1] (raw file):
I'm not sure would pylint catch the issue. For instance, you can use dict in both Python 2 & 3 and call items() on both, but on py2 it's going to create copies. So it will work, just slower. from builtins import * will import their dict on py2 which is a dict subclass that calls iteritems on py2 (among other things) thus making behavior consistent across versions.

And no test would catch a missing dict import since everything would still work (just slower on py2). I really think we need to import everything in all files.

---

Comments from the review on Reviewable.io

[vheon]

Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

ycmd/responses.py, line 21 [r1] (raw file):
I'm ok with using all imports from python-future regardless of the uses.

---

Comments from the review on Reviewable.io

[micbou]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

ycmd/responses.py, line 21 [r1] (raw file):
It looks like this is the recommended way. I am fine with it then. Maybe we should add a comment?

---

Comments from the review on Reviewable.io

[vheon]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

ycmd/responses.py, line 21 [r1] (raw file):
Yeah that was the page I was referring to. Do you want to put a comment on every file?

---

Comments from the review on Reviewable.io

[micbou]

ycmd/responses.py, line 21 [r1] (raw file):
At least, separate it from other imports.

---

Comments from the review on Reviewable.io

[Valloric]

Review status: 4 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

ycmd/responses.py, line 21 [r1] (raw file):
Separating from other imports with an empty line is fine (and done); adding a comment would be a good idea in general, but probably not in this instance since we'd then have to have it in every single file.

---

Comments from the review on Reviewable.io

[micbou]

---

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from the review on Reviewable.io

[vheon]

@homu r+

---

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from the review on Reviewable.io

[homu]

📌 Commit 25f8ce8 has been approved by vheon

[homu]

⌛ Testing commit 25f8ce8 with merge 5505f44...

[puremourning]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

ycmd/responses.py, line 21 [r1] (raw file):
For the record, I think the always import so you don't have to remember is fine. I believe that is what one of the many magic port scripts does too

---

Comments from the review on Reviewable.io

[Valloric]

Travis is clogged a bit and enough rows have finished successfully for me to consider it safe to merge this sans-homu.