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

[WIP] Allow upgrade to Django 2.0 #422

Merged
merged 14 commits into from Mar 18, 2018
Merged

[WIP] Allow upgrade to Django 2.0 #422

merged 14 commits into from Mar 18, 2018

Conversation

inducer
Copy link
Owner

@inducer inducer commented Jan 15, 2018

cc @dzhuang

Also jump onto new version of Celery.

Things to resolve:

  • Django 2.0 drops Py2
  • Update install docs: Celery 4 requires a message broker, e.g. redis or rabbitmq

@inducer
Copy link
Owner Author

inducer commented Jan 15, 2018

@dzhuang I'll wait with this until you have a chance to test it/give the go-ahead.

@inducer
Copy link
Owner Author

inducer commented Jan 15, 2018

The unicode fixes in ce58e1b are probably worth having no matter what.

@inducer
Copy link
Owner Author

inducer commented Jan 15, 2018

Heh. Kind of forgot about the biggest part of this. Django 2.0 is Py3 only. Personally, I've been on Py3 in production for a while, and I'm ready to let Py2 go.

Either way, we would need some CI engineering.

How about you?

@inducer inducer changed the title Alow upgrade to Django 2.0 [WIP] Alow upgrade to Django 2.0 Jan 15, 2018
@inducer inducer changed the title [WIP] Alow upgrade to Django 2.0 [WIP] Allow upgrade to Django 2.0 Jan 16, 2018
@dzhuang
Copy link
Contributor

dzhuang commented Jan 16, 2018

Got it.

@dzhuang
Copy link
Contributor

dzhuang commented Jan 22, 2018

@inducer Can you merge master into this? Thanks.

@inducer
Copy link
Owner Author

inducer commented Jan 22, 2018

Done.

@dzhuang
Copy link
Contributor

dzhuang commented Jan 22, 2018

Can you review this? Thanks. inducer/django-bootstrap3-datetimepicker#7

@inducer
Copy link
Owner Author

inducer commented Jan 23, 2018

Done.

@codecov
Copy link

codecov bot commented Jan 23, 2018

Codecov Report

Merging #422 into master will decrease coverage by <.01%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #422      +/-   ##
==========================================
- Coverage    72.9%   72.89%   -0.01%     
==========================================
  Files          45       45              
  Lines       11126    11128       +2     
  Branches     2069     2069              
==========================================
+ Hits         8111     8112       +1     
- Misses       2497     2499       +2     
+ Partials      518      517       -1
Impacted Files Coverage Δ
course/content.py 70.98% <ø> (ø) ⬆️
course/versioning.py 62.26% <ø> (ø) ⬆️
relate/settings.py 76.92% <44.44%> (-3.35%) ⬇️
course/validation.py 53.46% <0%> (+0.35%) ⬆️

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 397510b...a262b8d. Read the comment docs.

@dzhuang dzhuang force-pushed the master branch 2 times, most recently from d501a29 to 3c0900e Compare February 7, 2018 15:39
@inducer
Copy link
Owner Author

inducer commented Mar 16, 2018

@dzhuang How do you feel about this? I'd prefer to get this in this weekend.

For now, we're just allowing Django 2, so it doesn't rule out Py2 just yet. Plus my Celery is slightly broken in production, and I'd like to only re-debug it once. (rather than twice--now and again when we hop to Celery 4.)

@inducer
Copy link
Owner Author

inducer commented Mar 16, 2018

Either way, we would need some CI engineering.

I no longer believe that's true. Py2 will just select Django 1.11 automatically.

@dzhuang
Copy link
Contributor

dzhuang commented Mar 17, 2018

I'm ok with this, but the docs were not ready yet.

Sorry for late response (until I finally found a way to access my gmail).

@inducer
Copy link
Owner Author

inducer commented Mar 18, 2018

@dzhuang I'm using this brach to also switch to pipenv (which I seem to rather like for predictable deployment). Could you give this a try and see what you think?

@dzhuang
Copy link
Contributor

dzhuang commented Mar 18, 2018

You can merge it once you think it's ready (I won't have time to test this in a couple of hours). I'll complain if it failed for me. :)

@inducer
Copy link
Owner Author

inducer commented Mar 18, 2018

OK, thanks! I'll at least make sure I keep the CIs working.

@inducer
Copy link
Owner Author

inducer commented Mar 18, 2018

Wow, seriously, forget Pipenv. What a waste of time.

@dzhuang
Copy link
Contributor

dzhuang commented Mar 18, 2018

I see there is a long que in appveyor ci (due to hang of previously failed build). I think you can manually cancel those build to see whether the latest build works.

@inducer
Copy link
Owner Author

inducer commented Mar 18, 2018

In it goes.

@inducer inducer merged commit efdb51f into master Mar 18, 2018
@dzhuang
Copy link
Contributor

dzhuang commented Mar 18, 2018

Finally, thanks. Let's wait and see :)

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.

None yet

2 participants