Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move global functions from include/ to class methods in src/ #3878

Closed
7 tasks done
annando opened this issue Nov 7, 2017 · 116 comments
Closed
7 tasks done

Move global functions from include/ to class methods in src/ #3878

annando opened this issue Nov 7, 2017 · 116 comments
Labels
Enhancement Junior Jobs These are issues we think are good for starting to work with the Friendica code base

Comments

@annando
Copy link
Collaborator

annando commented Nov 7, 2017

We already are having some files in /include that are containing classes that we could move without a problem to /src.

This includes diaspora.php, dfrn.php and ostatus.php and possibly some more.

Tracking document: https://ethercalc.org/friendica_classes

Please update it with your planned changes.

Per @MrPetovan

So that everything is clear: in Model should only be static classes that interact with the DB with the same name as a database table.
In Object should be all the support classes that don't interact with any database table and shouldn't have the same name as a database table.

Remaining files:

@annando annando added this to the 3.6 milestone Nov 7, 2017
@MrPetovan
Copy link
Collaborator

@zeroadam expressed his interest in working on this issue.

@MrPetovan MrPetovan added Enhancement Junior Jobs These are issues we think are good for starting to work with the Friendica code base labels Nov 7, 2017
@annando
Copy link
Collaborator Author

annando commented Nov 7, 2017

So now we should collect a list of files and their best class paths.

@MrPetovan
Copy link
Collaborator

Periodic reminder: https://ethercalc.org/friendica_classes

@annando
Copy link
Collaborator Author

annando commented Nov 7, 2017

Currently we cannot move every file on that list. the dba.php and Photo.php for example do contain class methods and normal functions. We had to split them before moving them.

Files where it should work are:

  • dbm.php
  • dfrn.php
  • diaspora.php
  • NotificationsManager.php

Attention:
If these are moved, then they need a use dba;and use dbm; as well, if these class methods are used in the files.

But if dbm.php is moved, then the use dbm; in all files under src has to be changed to the full path as well.

@MrPetovan
Copy link
Collaborator

PHP Fatal error: Uncaught Error: Class 'Friendica\Protocol\Crypto' not found in src/Protocol/DFRN.php:1299

I'm on it.

@annando
Copy link
Collaborator Author

annando commented Nov 9, 2017

Next files could be:

  • cache.php
  • DirSearch.php
  • Emailer.php
  • ForumManager.php
  • Smilies.php
  • xml.php

Relocating these should be less harder, since they aren't as much included as dbm.php.

annando added a commit that referenced this issue Nov 9, 2017
annando added a commit that referenced this issue Nov 9, 2017
@zeroadam
Copy link

zeroadam commented Nov 9, 2017

So, will this issue stay open for all future include/ to src/ work to reference?

@MrPetovan
Copy link
Collaborator

Indeed, and already a big thank you for your work, it's much appreciated!

@zeroadam
Copy link

zeroadam commented Nov 9, 2017

Absolutely welcome! Glad to finally be able to start contributing, and glad to continue to do so!!!

@annando
Copy link
Collaborator Author

annando commented Nov 9, 2017

You make @MrPetovan very happy with this work.

@MrPetovan
Copy link
Collaborator

So happy I stay up most of the night to track fatal errors with a third-party library! 😅

(I truly am happy)

@rabuzarus
Copy link
Collaborator

rabuzarus commented Nov 9, 2017

BTW: If someone has time and is very motivated; the method names in classes should be renamed to camelCase.
e.g. DBM::isResult( ) (instead of DBM::is_result( ))

@annando
Copy link
Collaborator Author

annando commented Nov 9, 2017

@rabuzarus I suggest that we are doing this after we moved all possible classes to /src. Then someone can have a look at the method under this folder and can clean this up.

@zeroadam
Copy link

zeroadam commented Nov 9, 2017

I was also noticing this, but figured @annando wouldn't want all these changes in this particular work.

@rabuzarus
Copy link
Collaborator

Just thought, while breaking addon for friendica master, we should also rename the method names in this develop cycle. So we don't need to break it again in the next cycle.
But this was meant as suggestion.

@nupplaphil
Copy link
Collaborator

This one is finished, yipeah! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Junior Jobs These are issues we think are good for starting to work with the Friendica code base
Projects
None yet
Development

No branches or pull requests

6 participants