Introduce default format setting.

[proofit404]

Good evening,

The reason I'm interested in this pull request to be landed:

We frequently use model field from third-party library which can't be handled by core Django serializers.

I wrote custom serializer, not big deal. But now I need to mention custom format name in every call to register decorator.

Would be nice to set it once and forget about it :smile:

Best regards, Josiah.

[etianen]

Hi! This is an excellent PR, and I appreciate you taking the time to add documentation and tests. :+1:

I have one concern with this - historically I've avoided adding any settings to django-reversion, since there is only one entry-point to configuration (reversion.register). What you're trying to do here (default settings for reversion.register), could be acheived by wrapping reversion.register with your own register function. If you use the admin integration, you can likewise override the reversion_register in a common base class.

That being said, I don't want to be dogmatic about this. If you think there's an argument for adding the first django-reversion setting, and it suits your own use-case better, then I'm happy to change my mind. Or maybe you hadn't considered wrapping the register function, and prefer that approach? I'm very happy to discuss it further here, and happy to be pursuaded! :stuck_out_tongue_closed_eyes:

If we do go ahead with adding a setting, this PR is very ready to merge, and I've no requested changes. :heart:

[proofit404]

Hello,

We have about 300 models decorated with vanilla @register call.

Intention behind this pull request was my confusion.

There is no easy way to change default behavior in the existing code base.

We were forced to touch 300 decorator calls.

Be it either new value for the argument or new decorator function.

None of these changes does not follow DRY principle.

Of course, it's only a nice to have option if someone would encounter similar situation.

If you agree to introduce settings to the library, I would like to spend a little bit more time on this PR.

First of all, it could be a good idea to make settings variable a mapping similar to approach in DRF.

Also, I would like to write a little tutorial how we solved our problem from cover to cover with datamigration for existed model instances.

What do you think?

Best regards, Josiah.

[proofit404]

Any decision?

[etianen]

Sorry, yes. The settings dict is a good idea, as is the idea of a default format setting.

On Fri, 9 Dec 2022 at 09:36, Josiah Kaviani @.***> wrote:

Any decision?

— Reply to this email directly, view it on GitHub https://github.com/etianen/django-reversion/pull/928#issuecomment-1344073989, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEKCBUYELHZESWMZHXKWLWML4SJANCNFSM6AAAAAASGXT3FU . You are receiving this because you commented.Message ID: @.***>

[etianen]

I would merge that

On Fri, 9 Dec 2022 at 18:37, Dave Hall @.***> wrote:

Sorry, yes. The settings dict is a good idea, as is the idea of a default format setting.

On Fri, 9 Dec 2022 at 09:36, Josiah Kaviani @.***> wrote:

Any decision?

— Reply to this email directly, view it on GitHub https://github.com/etianen/django-reversion/pull/928#issuecomment-1344073989, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEKCBUYELHZESWMZHXKWLWML4SJANCNFSM6AAAAAASGXT3FU . You are receiving this because you commented.Message ID: @.***>

[proofit404]

Good afternoon,

I added application settings class to handle user settings module for all register arguments.

If you are happy with intended design and chosen implementation, I'll start writing documentation for it.

Thanks!

[proofit404]

If there is no outstanding code issues, I would starting to write documentation.

[etianen]

I'm very happy with the code! Please do go ahead with docs.

[umairahmed515]

@etianen when are we going to merge this PR?

[proofit404]

I didn't updated pull request yet.

We have a newborn, so I highly limited in time.

I'm still interested to finish this contribution.