Add Django v1.11, 2.0 support #33

grahamu · 2018-02-14T06:44:43Z

Drop Django 1.8, 1.9, 1.10, and Python 3.3 support
Add URL namespacing
Move documentation into README.md and standardize layout
Convert CI and coverage to CircleCi and CodeCov
Add PyPi-compatible long description
Add migration checking in test suite

Reviewer Notes

Verify BSD license as seen in setup.py and README.md.
Verify on_delete=models.CASCADE or models.SET_NULL. Pretty sure I got this right, but sure would be nice to double check.
Verify documentation. Expanded and copied from separate files and converted to Markdown in README.md.
tox.ini now contains migration check
Updated the version to 1.0.0. Following convention used in Pinax for such a large change to supported Django/Python configurations.
Updated the classifier to "Development Status :: 5 - Production/Stable" from "3 - Alpha". I'm not certain that is warranted here and would appreciate your thoughts.

* Drop Django 1.8, 1.9, 1.10, and Python 3.3 support * Add URL namespacing * Move documentation into README.md and standardize layout * Convert CI and coverage to CircleCi and CodeCov * Add PyPi-compatible long description

update changelog

codecov · 2018-02-14T06:49:54Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d7336ba). Click here to learn what that means.
The diff coverage is 95.94%.

@@            Coverage Diff            @@
##             master      #33   +/-   ##
=========================================
  Coverage          ?   82.05%           
=========================================
  Files             ?       15           
  Lines             ?      758           
  Branches          ?      110           
=========================================
  Hits              ?      622           
  Misses            ?      102           
  Partials          ?       34

Flag	Coverage Δ
#py27dj111	`82.05% <95.94%> (?)`
#py34dj111	`82.05% <95.94%> (?)`
#py34dj20	`82.05% <95.94%> (?)`
#py35dj111	`82.05% <95.94%> (?)`
#py35dj20	`82.05% <95.94%> (?)`
#py36dj111	`82.05% <95.94%> (?)`
#py36dj20	`82.05% <95.94%> (?)`

Impacted Files	Coverage Δ
formly/fields.py	`72.72% <ø> (ø)`
formly/forms/widgets.py	`70.58% <ø> (ø)`
formly/receivers.py	`0% <0%> (ø)`
formly/callbacks.py	`66.66% <100%> (ø)`
formly/views/results.py	`100% <100%> (ø)`
formly/forms/design.py	`89.36% <100%> (ø)`
formly/models.py	`82.01% <100%> (ø)`
formly/utils/views.py	`90% <100%> (ø)`
formly/views/run.py	`55.26% <100%> (ø)`
formly/forms/run.py	`55% <100%> (ø)`
... and 2 more

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 d7336ba...b5ef88d. Read the comment docs.

grahamu · 2018-02-14T06:56:40Z

Hi @paltman and @jacobwegner! This PR supports MGH desire to use Django 2.0. I've applied many of the lessons learned in Pinax here, but there were a lot of changes so I'd appreciate a review from one or both of you when possible. MGH upgrade is blocked pending this update. Thanks in advance!

jacobwegner

@grahamu thanks for your work here; I had started some changes to linting / CI in another branch but hadn't made it as far.

I think the changes to classification / release to 1.0 are long overdue, and it fits in that the URL namespace is also a backward-incompatible change.

I left a few small comments on some of the linting scripts needing to be updated from expecting a pinax directory structure to formly.

I'm curious as to why the tests show as passing when there was clearly an error; perhaps a return code is getting silenced:

https://circleci.com/gh/eldarion/formly/9

Once the linting / coverage changes are made, I think this is good to go.

jacobwegner · 2018-02-14T15:21:11Z

setup.py


 setup(
    author="Patrick Altman",
    author_email="paltman@eldarion.com",
-    description="a dynamic form generator",
+    description="A forms/survey generator for dynamically constructor multi-page surveys",


perhaps "for dynamically constructed multi-page surveys"?

jacobwegner · 2018-02-14T15:21:49Z

tox.ini

+skip_glob=**/*/migrations/*
+
+[coverage:run]
+source = pinax


pinax --> formly

jacobwegner · 2018-02-14T15:22:02Z

tox.ini

+    coverage report -m --skip-covered
+
+[testenv:checkqa]
+commands =


pinax --> formly

jacobwegner · 2018-02-14T15:22:48Z

tox.ini

 max-complexity = 10
-exclude = formly/migrations/*,docs/*
+exclude = **/*/migrations/*


I think this exclusion glob might need to change to pick up formly/migrations

jacobwegner · 2018-02-14T15:22:54Z

tox.ini

+known_third_party=appconf
+sections=FUTURE,STDLIB,DJANGO,THIRDPARTY,FIRSTPARTY,LOCALFOLDER
+include_trailing_comma=True
+skip_glob=**/*/migrations/*


formly/migrations

grahamu · 2018-02-14T16:27:38Z

Thanks so much @jacobwegner, good catches! Since this code is used in production I'll examine expected test failures in more detail, and bump test coverage as needed.

plac8

grahamu · 2018-02-14T22:35:44Z

@jacobwegner

I'm curious as to why the tests show as passing when there was clearly an error; perhaps a return code is getting silenced

My testing shows flake8 xyz produces no error, even though xyz does not exist. I've fixed the pinax references and the exclusion globs. Now going after better test coverage.

Further disclosure: The documentation has been converted to single README.md file, no longer published to ReadTheDocs. Hopefully that's not a problem.

jacobwegner · 2018-02-14T22:39:37Z

@grahamu yep; saw the docs change and am +1

grahamu · 2018-02-15T00:42:14Z

@jacobwegner Since we're updating formly urls with namespacing and legacy projects will need to update all these references anyway, what do you think about simplifying the url names, removing the dt_ and rt_ prefixes? From what I can tell, those prefixes indicate Design and Run, but none of the following urlname strings conflict.

Older version: formly_dt_page_create
Updated version: formly:dt_page_create
Proposed version: formly:page_create

The dt_ and rt_ prefixes don't add anything AFAIK, YMMV. Certainly the proposed version is much more readable.

grahamu · 2018-02-15T01:00:28Z

@jacobwegner Page.move_up() and Page.move_down() do not work. My guess is they were copied from Field.move_up/down(), but Field doesn't have the same type of unique_together constraint that Page does. The error is

IntegrityError: UNIQUE constraint failed: formly_page.survey_id, formly_page.page_num

My guess is nobody has run into this because nobody has used these Page methods. I can hack together a solution by assigning a temporary unused page_num during the swap, or we can remove these methods entirely from the Page model and then restore correctly if/when needed. Your thoughts much appreciated, thanks.

Add test factories and helper mixins Add test dependencies

f-strings introduced in Python 3.6 unittest.mock introduced after Python 2.7

Improve test execution speed via settings.

Require Django 1.11+

jacobwegner · 2018-02-20T17:00:19Z

@grahamu I'm +1 on removing the broken page manipulation methods and to your URL changes (since we're disclosing the backwards incompatible changes as part of the release).

grahamu · 2018-02-20T18:04:23Z

Thanks @jacobwegner, appreciate your feedback!

Add page and field create view tests. Improve "duplicate survey" view test coverage.

Add commentary to `pragma: no cover` comment

Remove "View" suffix on name Fix missing context data when form is invalid

Remove .coveragerc in favor of single point of truth: tox.ini.

grahamu added 2 commits February 13, 2018 23:41

Add Django v1.11, 2.0 support

0870340

* Drop Django 1.8, 1.9, 1.10, and Python 3.3 support * Add URL namespacing * Move documentation into README.md and standardize layout * Convert CI and coverage to CircleCi and CodeCov * Add PyPi-compatible long description

Update README.md

6fc9eb9

update changelog

grahamu requested review from paltman and jacobwegner February 14, 2018 06:53

grahamu mentioned this pull request Feb 14, 2018

Fix migrations #19

Closed

jacobwegner requested changes Feb 14, 2018

View reviewed changes

Fix CI configuration

a504c0b

plac8

grahamu added 12 commits February 15, 2018 09:48

Add model tests

913072d

Add test factories and helper mixins Add test dependencies

Remove "dt_" and "rt_" urlname prefixes

b632906

plac8

54f6dd5

Remove f-strings; import mock

eacf3c4

f-strings introduced in Python 3.6 unittest.mock introduced after Python 2.7

Add doc section for Authentication Backend use

24d472b

fix typo

33e614b

Add settings required to test views

853148d

Improve test execution speed via settings.

Add django-bootstrap-form test requirement

0fe1c47

Require Django 1.11+

Add design view tests

2e8f2c4

Re-sort imports

70466c8

Fix content decoding

9d97b5b

Update test tool requirements

fe18127

grahamu mentioned this pull request Feb 21, 2018

Replace form field widgets with a class, not class instance #34

Merged

Merge branch 'master' into django-20

fc8d3ff

grahamu added 20 commits February 21, 2018 18:39

Remove unused Page.move_up/move_down methods

9d8939e

Add Field method tests

e6a7ba0

Rename view

d0bd49e

Add view tests

cc1ac82

Add page and field create view tests. Improve "duplicate survey" view test coverage.

plac8

cd7b5a4

Update sorting config

3604fc9

Fix form instantiation error

f014b61

Remove pinax-theme-bootstrap template include

8633440

Fix Field save() method

67c007f

Add more view tests

867a87c

plac8

85b7941

Update docstrings

be398cc

Add commentary to `pragma: no cover` comment

Revise template for pinax-theme-bootstrap removal

06fd1f1

Add Survey.get_run_url() convenience method

bc43ba5

Add "home" url for testing

a1d8a81

Use modern login required mixin

77e7882

Rename deletion views

12fb4bb

Remove "View" suffix on name Fix missing context data when form is invalid

Add view tests

d6d3c1b

plac8

db1e047

Update test coverage configuration

b5ef88d

Remove .coveragerc in favor of single point of truth: tox.ini.

grahamu merged commit 64e31ed into master Feb 23, 2018

grahamu deleted the django-20 branch February 23, 2018 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Django v1.11, 2.0 support #33

Add Django v1.11, 2.0 support #33

grahamu commented Feb 14, 2018 •

edited

Loading

codecov bot commented Feb 14, 2018 •

edited

Loading

grahamu commented Feb 14, 2018

jacobwegner left a comment •

edited

Loading

jacobwegner Feb 14, 2018

jacobwegner Feb 14, 2018

jacobwegner Feb 14, 2018

jacobwegner Feb 14, 2018

jacobwegner Feb 14, 2018

grahamu commented Feb 14, 2018

grahamu commented Feb 14, 2018

jacobwegner commented Feb 14, 2018

grahamu commented Feb 15, 2018

grahamu commented Feb 15, 2018 •

edited

Loading

jacobwegner commented Feb 20, 2018

grahamu commented Feb 20, 2018

Add Django v1.11, 2.0 support #33

Add Django v1.11, 2.0 support #33

Conversation

grahamu commented Feb 14, 2018 • edited Loading

Reviewer Notes

codecov bot commented Feb 14, 2018 • edited Loading

Codecov Report

grahamu commented Feb 14, 2018

jacobwegner left a comment • edited Loading

Choose a reason for hiding this comment

jacobwegner Feb 14, 2018

Choose a reason for hiding this comment

jacobwegner Feb 14, 2018

Choose a reason for hiding this comment

jacobwegner Feb 14, 2018

Choose a reason for hiding this comment

jacobwegner Feb 14, 2018

Choose a reason for hiding this comment

jacobwegner Feb 14, 2018

Choose a reason for hiding this comment

grahamu commented Feb 14, 2018

grahamu commented Feb 14, 2018

jacobwegner commented Feb 14, 2018

grahamu commented Feb 15, 2018

grahamu commented Feb 15, 2018 • edited Loading

jacobwegner commented Feb 20, 2018

grahamu commented Feb 20, 2018

grahamu commented Feb 14, 2018 •

edited

Loading

codecov bot commented Feb 14, 2018 •

edited

Loading

jacobwegner left a comment •

edited

Loading

grahamu commented Feb 15, 2018 •

edited

Loading