MonitorField - Change default to None when the field is nullable

[gmcrocetti]

Problem

I recently came across a non-intuitive behavior, IMHO. The MonitorField has its default value set to now even though marked as nullable. Please, I'm open to hearing the opinion case this is expected behavior.

Solution

After catching up the discussion in #100 I decided to implement it. The discussed solution is to set its default value to None when null=True.

If might break backward compatibility at the edge case of someone marking the field as null=True and not expecting default to be None.

Commandments

• Write PEP8 compliant code.
• Cover it with tests.
• Update CHANGES.rst file to describe the changes, and quote according issue with GH-<issue_number>.
• Pay attention to backward compatibility, or if it breaks it, explain why.
• Update documentation (if relevant).

[foarsitter]

We should plan a deprecation path don't you think @gmcrocetti?

[gmcrocetti]

We should plan a deprecation path don't you think @gmcrocetti?

Yes, we should. Is there any major release in the near sight ?
Do you have other suggestion ?

[foarsitter]

My cup of tea: A DeprecationWarning when the behavior is encountered and an environment variable to enable the new behavior so we can release 4.5, removing both when 5.0 is released?

[gmcrocetti]

Humm...I initially liked the idea but I'm not so sure after getting my hands dirty. What if we instead split this new behavior in two steps:

1. Raise a deprecation warning - as you said - for 4.5.0 in this existing PR;
2. We keep the original issue and target it for 5.x.x - I open a new PR containing the original proposal.

I believe it will be better cause we keep the codebase clean and also don't fall into the trap of "remembering" to remove "dead code" added by the feature flag check.

WDYT @foarsitter ?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (6e7158b) to head (22015b8).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #577   +/-   ##
=======================================
  Coverage   98.56%   98.57%           
=======================================
  Files           6        6           
  Lines         768      772    +4     
=======================================
+ Hits          757      761    +4     
  Misses         11       11           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[foarsitter]

Very well @gmcrocetti, perfect solution!