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

Prepare CALC for migration to Django 1.10 #1544

Merged
merged 3 commits into from Apr 13, 2017

Conversation

Projects
None yet
3 participants
@toolness
Contributor

toolness commented Apr 13, 2017

This PR prepares CALC for migration to Django 1.10--by which I mean that it makes low-impact, backwards-compatible changes to ensure that CALC's test suite passes on Django 1.10, but it doesn't actually upgrade CALC's version of Django. This is because we're about to put CALC on hiatus and might not be able to address any problems that might arise from actually migrating. But we do want to make it easy to migrate if we (or another team that inherits this project) need to upgrade once Django 1.9 reaches end-of-life. This will make it easier to eventually make our way to 1.11 (#1346), which is the next long-term support (LTS) release.

This is coupled with the changes made in our fork of djorm-ext-pgfulltext at 18F/djorm-ext-pgfulltext#3, so if we merge this PR, we should also merge that one. (If you need to make local edits to that fork and test them with CALC, you can follow the instructions at #1466 (comment).)

@toolness toolness requested a review from jseppi Apr 13, 2017

@jseppi

jseppi approved these changes Apr 13, 2017

Seems good as long as those duplicated required=False lines are actually necessary.

@@ -20,12 +20,12 @@ class MyForm(forms.Form):
js = forms.FileField(widget=UploadWidget(
required=False,
existing_filename='boop.csv',
))
), required=False)

This comment has been minimized.

@jseppi

jseppi Apr 13, 2017

Contributor

Does it need this both here and up at line 21?

This comment has been minimized.

@toolness

toolness Apr 13, 2017

Contributor

To be compatible with both Django 1.9 and 1.10, yes. Once we migrate to 1.10, I think we can get rid of the required in the UploadWidget. Note that I think we're already doing this in the actual upload form (outside of the styleguide) because the FileField arg dictates the behavior of server-side validation while the UploadWidget arg dictates the client-side rendering of the widget (i.e. the HTML5 required attribute).

@toolness toolness merged commit a8eca7e into develop Apr 13, 2017

4 checks passed

codeclimate no new or fixed issues
Details
codeclimate/coverage 95.75%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@toolness toolness deleted the django-1.10 branch Apr 13, 2017

@NoahKunin NoahKunin removed the in progress label Apr 13, 2017

@jseppi jseppi referenced this pull request Apr 14, 2017

Merged

Tag and release v2.7.0 #1545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment