Mailcow-dockerized: IMHO: Code Quality

[patschi]

Hi coMOOOunity.

this issue is basically just sort of a feedback from my side - my two cents. I don't want to blame anyone, nor saying that the code is awful or so - mailcow just works fine, does its job great and I'm very appreciating the work from everyone contributing to this project.

But in my opinion there should be some "rules" or "recommendations" for new PRs to keep working on the quality of the code a bit - for having a clean, fast and reliable working mailcow instance.

IMHO following points might play a big role in the code quality of mailcow:

• The phpfpm container uses PHP 7.2, so we should take advantage of PHP 7-syntax, functions and possibilities as much as possible.
• Using stricter === matches in IF-statements, rather than just == for e.g. performance reasons
• Using 1-tab to indent code-lines than just two spaces. In my opinion that's much more casual, looks cleaner and is a lot easier to read. A strict(er) coding guideline would be a good idea.
• Going over to a class-oriented programming style, than just using functions everywhere to get a more modular code-base. (Probably for some plugin-like feature in the far future?)
• Using a separated branch for some beta/testing-code, before pushing/merging it into the master branch (which should be the stable branch).
• Maybe also having a 4-eyes principle for merging code into the master branch, just like other bigger open source projects has

Thanks for taking time reading this! :)

[andryyy]

Hi,

I am fine with everything except for the 1-tab indent. :-)

I hate to use OOP in PHP for small projects. mailcows UI was very basic in the beginning. I agree it would be better to use OOP today, but who the heck has the time to change this now? :-( I would accept any PR that introduces OOP and also makes use of the advantages. I don't really want to copy it 1:1 into OOP style and just make the code bigger without benefit. :-/

[patschi]

In my opinion indention using 2-spaces looks a bit ugly, e.g. compared to when using 1-tab way like Nextcloud does in their code-base: https://github.com/nextcloud/server/blob/master/core/Controller/LoginController.php

I know that drastically changing the whole code base is not something which is done within a few days, as this is a long-ongoing process. But to get closer to a cleaner code we should take a more stricter look on new contributed code :)

Beside that, just something what came in my mind right now: What about using Bountysource to (crowd)fund features/enhancements? Again, Nextcloud as an example: https://www.bountysource.com/teams/nextcloud

[mxschmitt]

Hi there,

i would also think, that spaces (2 or 4) are much more established than tabs.
Another idea would be to lint the source code in the CI process, like I did it here in this repository by using phpcs. That should break the CI pipeline per default, once the source code is not in the correct "format".
But before that, the complete source code should be in a given styleguide and be OOP.

[develth]

There is a reason why https://www.php-fig.org/psr/psr-2/ exists ;)

[andryyy]

I will close it as duplicate of #708 - I reopened the other issue.