Alright, I'm still not 100% convinced it wouldn't be best having them as separate methods, but for the sake of moving things along I'll implement it.
One thing that's not quite clear: you mentioned adding a backwards compatibility warning, is this the same as a RemovedInDjangoXXWarning? Should I follow the documentation at [1] for this implementation? [1] https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#deprecating-a-feature Cheers, Adrian On Saturday, March 5, 2022 at 7:30:18 PM UTC+1 f.apo...@gmail.com wrote: > Hi Adrian, > > On Saturday, March 5, 2022 at 5:13:14 PM UTC+1 ator...@redhat.com wrote: > >> - Existing RemoteUserBackend implementations won't need to change >> signatures whenever backwards compatibility is removed >> - RemoteUserBackend implementations won't need to do anything in order to >> support Django versions in which the feature doesn't exist (e.g. 3.9) and >> versions in which the feature exists and is not backwards-compatible (e.g. >> 5.1) >> > > While this is true, the "migration" path for implementors is simply to > change the function to something like: > > ``` > def configure_user(self, request, user, created=False): > if not created: return > … do whatever you already did > ``` > > and it will stay compatible with current behavior and the new behavior. > > >> - The code footprint within Django, not counting documentation and tests, >> is like 3 LOC >> > > I do understand that, but I don't think it is as simple as that. With the > separated methods (unless we pass created to synchronize_user as well) > you'd do synchronization always even when the user was just created even > though configure_user could take care of that in one go. > > - Anyone who wants to extend a RemoteUserBackend implementation can easily >> and cleanly extend/replace the synchronization and initial setup >> independently of each other, if everything is done in the same method this >> can get messy >> > > I do not really think this will be the case since those methods are empty > by default… Those backends are so simple that at some point one can simply > write their own if subclassing might become to much of a hazzle. > > >> def configure_user(self, request, user, created): >> if created: >> user = self.initial_configuration(request, user) >> user = self.synchronize(request, user) >> return user >> >> Which is the same as having two separate methods for initial >> configuration and synchronization, but with extra steps. >> > > I do not think this will be common though. I rather think that usually one > would do the same thing on creation and on sync. > > >> Have a good weekend >> > > Thanks, you as well! > > Florian > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/cbb24085-a610-44ec-a5df-f58e6ab06f0cn%40googlegroups.com.
