search

nextcloud / user_external

👥 External user authentication methods like IMAP, SMB and FTP
https://apps.nextcloud.com/apps/user_external
108 stars 64 forks source link

optimise PSR4 and autoloading #57

Closed violoncelloCH closed 2 years ago

violoncelloCH commented 5 years ago

@violoncelloCH we should discuss PSR4 and autoloading at some point. This isn't necessary anymore if the class files are named correctly :rocket:

_Originally posted by @ChristophWurst in https://github.com/nextcloud/user_external/pull/37_

ref: https://docs.nextcloud.com/server/stable/developer_manual/app/classloader.html#psr-4-autoloading

ChristophWurst commented 5 years ago

Sure!

I would suggest to do it in a few consecutive steps:

  1. Fix the namespace from user_external to UserExternal
  2. Register the <namespace>UserExternal</namespace> in appinfo/info.xml
  3. Make sure each class has its own file
  4. Rename class files to their respective class name. class MyClass {} goes into MyClass.php (as opposed to my_class.php).

This should do the trick. Let me know if you have any questions :)

violoncelloCH commented 5 years ago

okay, I'll try to do this, when I've got some time for it :) only thing I'm concerned about are the class names for the authentication providers itself: they atm look like OC_User_IMAP. Would we need to change those to CamelCase style ones too? If so, this would force all users to update their configuration :/ Or we would need to migrate/update the configs automatically... would this be doable? Or would we maybe better change to the new way of how to register backends in the same step, so we only need to migrate one time? (sorry, I don't have any experience how this works)

ChristophWurst commented 5 years ago

You could rewrite the config automatically in a repair step. I think this should work just fine.

fskale commented 4 years ago

I'm not really into PHP and not familiar with Class-naming conventions in PHP as opposed to Perl. Nevertheless i'm able to read the code and found the regression patch on the owncloud github repository. For me, my hotfix works, and therefore you should provide the fix until you've rewritten your classes. @ChristophWurst already provided details, so IMHO, he can do it best.

ChristophWurst commented 4 years ago

@ChristophWurst already provided details, so IMHO, he can do it best.

hehe if only there was no other work to do :)

violoncelloCH commented 2 years ago

I think this is all addressed in #191... (major, breaking upgrade which requires users to manually change their configs though) can you confirm and close this @ChristophWurst ?