search

openfoodfacts / openfoodfacts-server

Open Food Facts database, API server and web interface - 🐪🦋 Perl, CSS and JS coders welcome 😊 For helping in Python, see Robotoff or taxonomy-editor
http://openfoodfacts.github.io/openfoodfacts-server/
GNU Affero General Public License v3.0
657 stars 384 forks source link

Choose a formatting style guide for the project #2044

Open VaiTon opened 5 years ago

VaiTon commented 5 years ago

Introduction

It's important to use a tool to auto-format the code not only because it improves readability by a lot but also because it makes bug fix easier and once devs are used to that style also modifying code becomes easier.

Proposal

That's why I propose we use perltidy with the gnu style configuration to auto-format the code and test it with TravisCI.

Any suggestion are welcome expecially about deciding the style config the formatter should follow :-)

VaiTon commented 5 years ago

I would like to hear your opinion @openfoodfacts/openfoodfacts-server

CharlesNepote commented 5 years ago

We use .editor config file. Isn't it ok? Did you read: https://en.wiki.openfoodfacts.org/Software_Development ?

VaiTon commented 5 years ago

Yeah .editorconfig is ok. The problem is that for perl it doesn't enforce any coding style and so any consistency. In a lot of files you will see formatting inconsistencies like these.

Here the method is called without leaving space inside the parenthesis. https://github.com/openfoodfacts/openfoodfacts-server/blob/855a82868da529290f7947358b0da70a801c91fe/t/food_normalize_serving_size.t#L24

Here we left a space right before the parenthesis. Also the method call is closed on next line. https://github.com/openfoodfacts/openfoodfacts-server/blob/855a82868da529290f7947358b0da70a801c91fe/t/additives_tags.t#L37-L38

hangy commented 5 years ago

I think it could be a good idea to decide on a code style and to enforce it with Test::PerlTidy during CI.

Unsure about the "best" style to choose. Have you tested which options cause the fewest changes to the current source code? It might also be good to not report code style variations as errors right away, because re-formatting a complete codebase in one go isn't a good idea and will break most open PRs.

VaiTon commented 5 years ago

Maybe the GNU style would cause the fewest changes but it indeed they are a lot.

CharlesNepote commented 4 years ago

Is there a way to "convert" files only if they are modified?

Would the first step be to validate all PRs?

hangy commented 4 years ago

Would the first step be to validate all PRs?

I like the idea! With GitHub Actions this could be possible, as demonstrated by https://github.com/lots0logs/gh-action-get-changed-files/blob/master/entrypoint.js One only potential problem could be the size of ProductOpener modules. If we changed a line in Display.pm, the other thousands of lines of code would also be run by perltidy.