Have code formatting guideline

[ldidry]

Discussion will happen on [email protected] mainling list.

Right now, we have minimal vim modelines:

• indent with 4 spaces, no tabs

But there is a lot of other coding style stuff that could (should) be discussed, in order to have a coherent coding style in all codebase, like:

• max line width
• to put brace on the same line than the if (while, for, etc)
• etc

To close this issue, we need to:

• agree on the coding style we want
• write a .perltidyrc file
• have a perltidy test in continuous integration, to be sure that PR are style compliant
• document the coding style guideline in human language on https://github.com/sympa-community/sympa/wiki

[ldidry]

In fact, there is already a perltidyrc in https://github.com/sympa-community/sympa/blob/sympa-6.2/doc/dot.perltidyrc.

Let's use it as the base of the coding style we want.

[ikedas]

Hi @ldidry,

Basically, I think you’d be better to create your fork of master repo, create a branch and submit PR to propose changes. (It is not the rule: Currently we have no consensus about branching/forking, but I feel it is better in many cases). See GitHub Help for details. Sorry for unnecessary comment.

Besides, I’ve not yet had confidence to force a style to coders. Because:

• If we decided to reformat sources by perltidy-ing regularly, test might not be required.
• As far as I know, formatting behavior of Perl::Tidy changed release by release (please check results with some versions of perltidy). Even if coders reformatted their changes by themselves, they might not be accepted by the test.

Additional comment:
Coding style may include something not covered by reformatting. For example,

• Naming rules: Symbols including names of packages, variables, methods, and built-in variables (either to use short or long).
• Qualification of symbols: Either to use exported names or not (if not, always use qualified names for symbols in external modules).
• Required and allowed pragma.

Regards,
— Soji

[ldidry]

If we decided to reformat sources by perltidy-ing regularly, test might not be required.

Well, in fact, we might: having this test allows to use it in Travis, so that you can check that a PR is perltidy-compliant. It's not a non-acceptance flag for the PR, of course, but by seeing this, we would be able to fix the tidying right after we accept the PR

As far as I know, formatting behavior of Perl::Tidy changed release by release (please check results with some versions of perltidy). Even if coders reformatted their changes by themselves, they might not be accepted by the test.

As I created a cpanfile to install dev modules (for Test::PerlTidy at first, then I saw another module for another test), we could enforce the version of PerlTidy when installing it to hack on Sympa, so every Sympa developper would (normally) use the same version and have the same behavior

Coding style may include something not covered by reformatting. For example,

You're absolutely right. What do you think about I rename this issue to "Have code formatting guideline" and create an issue that:

• is named "Have coding style guideline" (kind of a meta-issue)
• references this issue
• talks about the other code style things (pragmas, naming rules, etc.) (or maybe that point could be a new issue, referenced in the meta-issue)

[ikedas]

If we decided to reformat sources by perltidy-ing regularly, test might not be required.

Well, in fact, we might: having this test allows to use it in Travis, so that you can check that a PR is perltidy-compliant. It's not a non-acceptance flag for the PR, of course, but by seeing this, we would be able to fix the tidying right after we accept the PR

As far as I know, formatting behavior of Perl::Tidy changed release by release (please check results with some versions of perltidy). Even if coders reformatted their changes by themselves, they might not be accepted by the test.

As I created a cpanfile to install dev modules (for Test::PerlTidy at first, then I saw another module for another test), we could enforce the version of PerlTidy when installing it to hack on Sympa, so every Sympa developper would (normally) use the same version and have the same behavior

I understood. I agree to implementing perltidy test and see what will happen.

Coding style may include something not covered by reformatting. For example,

You're absolutely right. What do you think about I rename this issue to "Have code formatting guideline" and create an issue that:

• is named "Have coding style guideline" (kind of a meta-issue)
• references this issue
• talks about the other code style things (pragmas, naming rules, etc.) (or maybe that point could be a new issue, referenced in the meta-issue)

I agree too.

[ldidry]

• Issue renamed
• Meta issue created (Have coding style guideline #325)
• Perl::Tidy exact version enforced in cpanfile (I choose the 20180220 version, since it was the version I have, but I don't care if somebody prefers an other version)
• I added the test on Travis (I tested that before in an other branch, and now I will delete that old branch)

There is a PR waiting for all the current work to be merged into 6.2.x branch: #322

About the Travis test: should we let it in Travis or comment it, waiting for us to tidy up all files before re-enabling it? It now complains a lot (of course) and so isn't that useful. It will be useful when it will complain on introducing new untidy code. What do you think?

[ikedas]

One more comment.

I concepted that documents on sympa-community.github.io will be a "documentation set" for users, and documentaion for developers would be on separate place (in its nature, it will tend to fluid and more or less inconsistent). How about using wiki built in GitHub for documentation about development?

[ldidry]

I think it would a good thing, yes 🙂

[ldidry]

Wiki created: https://github.com/sympa-community/sympa/wiki

[ikedas]

Thanks. I'll let sympa-developpers people know.

[ikedas]

I see this issue may be closed now: The last task may be taken over to the new issue.