jazzband / django-formtools

A set of high-level abstractions for Django forms
https://django-formtools.readthedocs.io
BSD 3-Clause "New" or "Revised" License
794 stars 135 forks source link

Forced get_form() to kwargs-only #208

Closed claudep closed 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

Merging #208 (d4a423d) into master (eb9dec4) will increase coverage by 0.18%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
+ Coverage   93.01%   93.20%   +0.18%     
==========================================
  Files          11       11              
  Lines         530      530              
  Branches       67       67              
==========================================
+ Hits          493      494       +1     
+ Misses         23       22       -1     
  Partials       14       14              
Impacted Files Coverage Δ
formtools/wizard/views.py 94.29% <100.00%> (+0.33%) :arrow_up:

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 eb9dec4...d4a423d. Read the comment docs.

hramezani commented 2 years ago

Hi @claudep, Can you explain why do we need this change?

claudep commented 2 years ago

Well, it's less ambiguous and a bit easier when subclassing:

def get_form(self, *args, **kwargs):
    /* Here you never know if step is in *args or in **kwargs. */

So you are forced to use the get_form(self, step=None, **kwargs): signature. However, it might break some code, so I'm not sure now if it's really worth it.

hramezani commented 2 years ago

Yes, it will break some code I think. I am also not sure. I think we need to add an entry to change log if we are going to merge this

claudep commented 2 years ago

Let's close it.