docker-mailserver / docker-mailserver

Production-ready fullstack but simple mail server (SMTP, IMAP, LDAP, Antispam, Antivirus, etc.) running inside a container.
https://docker-mailserver.github.io/docker-mailserver/latest/
MIT License
14.69k stars 1.83k forks source link

Format scripts according to standard #1221

Closed erik-wramner closed 4 years ago

erik-wramner commented 5 years ago

The scripts are not formatted according to the standard and as a consequence there is a mix of indentation sizes, sometimes tabs are used but mostly spaces. It makes it harder to work with the code, in particular when blocks of code are copied/moved between files that use different conventions. In particular the start-mailserver.sh script uses tabs in some places and spaces in others and indents with 8 spaces, whereas the standard is to only use two spaces and no tabs.

Possible Fix

I can fix the scripts (or at least start-mailserver.sh to begin with), but as this will cause major changes I would like to do it at an agreed point in time when we try to avoid having outstanding work. Merging real changes when there are whitespace changes all over is not my idea of fun!

fbartels commented 5 years ago

One way to tackle this would be to add an .editorconfig config file into the repository. Not all editors support this standard out of the box, but this would be a way to define the expected behavior in this project.

https://editorconfig.org

Current output of ´eclint´ (xargs stops after the first command has failed, therefore only errors of ´setup.sh´ are shown):

$ git ls-files --exclude='*.sh' --ignored | xargs --max-lines=1 eclint check
setup.sh
    152:01 ❌ invalid indent size: 3, expected: 4       (EditorConfig indent_size  https://goo.gl/QtGB8R)
    153:01 ❌ invalid indent size: 5, expected: 6       (EditorConfig indent_size  https://goo.gl/QtGB8R)
    154:01 ❌ invalid indent size: 5, expected: 6       (EditorConfig indent_size  https://goo.gl/QtGB8R)
    276:01 ❌ invalid indent style: tab, expected: space
                                                         (EditorConfig indent_style https://goo.gl/8Qkrbr)
erik-wramner commented 5 years ago

Yes, but the problem with the big change and hence merge issues with any outstanding PRs remains. It would be much easier to do this when there are no or at least as few PRs waiting as possible. We also need to consider the new Debian branch and any changes that have been done there.

georglauterbach commented 4 years ago

I'll have a look at this. Should be about the right time.

georglauterbach commented 4 years ago

Done with #1619