Skip to content
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

Revert using highest possible pickle protocol #1164

Closed
wants to merge 2 commits into from

Conversation

micbou
Copy link
Contributor

@micbou micbou commented Jul 3, 2018

See discussion in PR #1108. Add a note that the version of the pickle protocol should be increased to 4 when Python 2.7 and 3.3 are dropped.


This change is Reviewable

@davidhalter
Copy link
Owner

@blueyed What do you think? I think my comments in #1108 still apply.

@codecov-io
Copy link

Codecov Report

Merging #1164 into master will increase coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1164      +/-   ##
==========================================
+ Coverage    92.6%   92.61%   +0.01%     
==========================================
  Files          57       57              
  Lines        6884     6882       -2     
==========================================
- Hits         6375     6374       -1     
+ Misses        509      508       -1
Impacted Files Coverage Δ
jedi/api/environment.py 78.53% <100%> (ø) ⬆️
jedi/evaluate/compiled/subprocess/__init__.py 75% <83.33%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4aad8b...59d0a5d. Read the comment docs.

@blueyed
Copy link
Contributor

blueyed commented Jul 3, 2018

I would say to keep it given that my plan work (#1108 (comment)).
IIRC the problem was to have the highest_pickle_protocol function available in the client - that's when I've started doing the PYTHONPATH PR (#1159).

@davidhalter
Copy link
Owner

I'll just leave this one open, let's see what's going to happen.

@davidhalter
Copy link
Owner

Since @blueyed found a solution for the problem in #1108, I think we can safely close this one.

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.

4 participants