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

Avoid incompatible *.dat files between Python 2 and 3 #188

Merged
merged 2 commits into from Sep 9, 2015

Conversation

@jun66j5
Copy link
Contributor

commented Aug 5, 2015

Proposed changes for #174.

In Babel 2.0, *.dat files use datetime.date class. However, pickle data format for the date class is incompatible between Python 2 and 3. Proposed changes remove uses of date class from *.dat files and add check for only babel.* classes in *.dat files.

@codecov-io

This comment has been minimized.

Copy link

commented Aug 5, 2015

Current coverage is 82.94%

Merging #188 into master will increase coverage by +0.02% as of 9a28d0b

@@            master    #188   diff @@
======================================
  Files           21      21       
  Stmts         3770    3774     +4
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           3126    3130     +4
  Partial          0       0       
  Missed         644     644       

Review entire Coverage Diff

Powered by Codecov

@sils

This comment has been minimized.

Copy link
Member

commented Aug 5, 2015

hey, the first lines of your commit message are too long and will be capped by git. Could you take a look at http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/#how-to-write-good-commit-messages and adjust the messages accordingly so when problems with that code arise we can use git blame effectively?

jun66j5 added 2 commits Aug 4, 2015

@jun66j5 jun66j5 force-pushed the jun66j5:issue174/workaround branch from 33bb6d7 to 6acf7ae Aug 5, 2015

@jun66j5

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2015

Okay. I revised the commit message.

@erickwilder

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2015

It's ok to me. @sils1297 any extra input here?

@sils

This comment has been minimized.

Copy link
Member

commented Aug 6, 2015

@erickwilder I'm not competent to approve this PR right now but it looks sane on first sight.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented Aug 6, 2015

We should add tests for this somehow.

@erickwilder

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2015

@mitsuhiko
Any suggestions on what kind of tests to add here in addition of the code added to test_core.py file?

@sils

This comment has been minimized.

Copy link
Member

commented Aug 25, 2015

@erickwilder @mitsuhiko might likely not reply when @'ed as he gets emails for every comment of babel implicitly. I suggest you ping him on IRC when things occur.

@vstinner

This comment has been minimized.

Copy link

commented Aug 28, 2015

The change is simple and includes a unit test, it looks good to me!

I need this fix because I also have the bug #174 on the OpenStack Horizon project.

erickwilder added a commit that referenced this pull request Sep 9, 2015
Merge pull request #188 from jun66j5/issue174/workaround
Avoid incompatible *.dat files between Python 2 and 3

@erickwilder erickwilder merged commit 471f8c8 into python-babel:master Sep 9, 2015

4 checks passed

codecov/patch 100.00% (min required 90.00%)
Details
codecov/project 82.94% (min required 80.00%)
Details
continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
shimizukawa added a commit to sphinx-doc/sphinx that referenced this pull request Sep 13, 2015
Avoid "2.0" version of Babel because it doesn't work with Windows env…
…ironment. Closes #1976.

see also:
* python-babel/babel#174
* python-babel/babel#188

Version spec syntax "babel>=1.3,!=2.0" is following PEP440: https://www.python.org/dev/peps/pep-0440/#version-exclusion and it works with pip 6.0 or later (I didn't check before pip 6.0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.