Manual talk:Coding conventions

Jump to navigation Jump to search

About this board

Operator placement on line break

5
Summary by Tgr (WMF)

Relaxed the text, PSR-12 style ("Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.")

Tgr (WMF) (talkcontribs)

The manual says "The operator separating the two lines should be placed at the end of the preceding line." That's poor advice (start-of-line operators are much easier to scan), and in my experience gets routinely ignored. Compare

return $a > 0 &&
    $a < 100 ||
    $a = 0 &&
    $b > 0;

with

return $a > 0
    && $a < 100
    || $a = 0
    && $b > 0;

for readability. Or

doStuffWith( $foo, $bar .
    $baz, $boom );

We should change the coding style to say start-of-line placement, which is what is used in practice in most of our code anyway.

Krinkle (talkcontribs)

I'd recommend taking it out of this page and incorporating it instead into the pages about specific programming languages as needed.

Jdforrester (WMF) (talkcontribs)

What Krinkle says. For good or ill, we have different conventions for JavaScript and PHP right now on operator placement.

Tgr (WMF) (talkcontribs)

Well, I can take it out from here and add to the PHP page; I'd have no idea what to add to JS though.

Tgr (WMF) (talkcontribs)

Relaxed the text, PSR-12 style ("Boolean operators between conditions MUST always be at the beginning or at the end of the line, not a mix of both.")

Formating of multiline if statements

2
Nikerabbit (talkcontribs)

What is the reason we recommend different format for ifs compared to functions. Here is what I mean:

function foo(
	$longVarName1,
	$longVarName2,
	$longVarName3,
	$longVarName3
) {
	...
}

$res = $obj->methodWithManyParameters(
	$longVarName1,
	$longVarName2,
	$longVarName3,
	$longVarName4
);

if ( $someLongThing !== $someOtherLongTail ||
	$future > $past ||
	$future === $now
) {
	...
}

# I would use:
if (
	$someLongThing !== $someOtherLongTail ||
	$future > $past ||
	$future === $now
) {
	...
}

It looks inconsistent to me.

Anomie (talkcontribs)

I can't say for sure. But personally, since the if ( prefix is universally short, the first condition doesn't get visually lost like it can if you were to do

$res = $obj->methodWithManyParameters( $longVarName1,
    $longVarName2,
    $longVarName3,
    $longVarName4
);
Reply to "Formating of multiline if statements"
ESanders (WMF) (talkcontribs)

There are some exceptions, such as switch statements, where the indentation of the cases are optional, so both of the below are fine.

This seems odd:

  1. In JS the indentation is required and enforced by eslint, so this isn't true in that case
  2. in PHP it seems odds to allow no indentation when every other type of braced block requires indentation

I think it would be simpler just to remove this exception.

Anomie (talkcontribs)

In MediaWiki core and deployed extensions there are 486 PHP files with indented cases and 93 with non-indented cases. 20 files have both types, and so are included in both counts.

Huji (talkcontribs)

Not sure how you did the math here, @Anomie:, but how about we change those 93 cases to be indented too, and then remove the exception as suggested by ESanders?

Anomie (talkcontribs)

find . ../wmf/extensions/ \( -name vendor -o -name node_modules -o -name .git \) -prune -o -name \*.php -exec perl -lnwe '$i=$1 if /^(\t+)switch/; if($i && /^${i}case /){ print $ARGV; last; }' {} \; for the non-indented, and the same with /^${i}\tcase / in the middle for indented.

Ed g2s (talkcontribs)
Reply to "Switch statements"
Anomie (talkcontribs)

(ugh, Flow)

@Krenair, Nemo bis, Krinkle: Let's discuss. I see three things in the section under dispute:

  • Messages
  • Comments
  • Documentation

There are other things that could be included but aren't explicitly listed:

  • Names of classes, functions, methods, parameters, variables, constants, etc.
  • Names of database tables, fields, etc.
  • Message keys
  • Non-localized exception and log texts

Messages shouldn't need dispute: Messages in en.json should use American English, since en-gb.json and en-ca.json exist for other varieties, while en-us.json doesn't because we use .

As for the rest, consistency in documentation (including doc-comments) within an area would make a better impression on those reading the documentation, and in thing-naming makes it slightly easier to remember the name of a thing if you're not using an IDE with auto-complete, but generally it's all going to be understandable by anyone who understands English and IMO doesn't need a major battle over it.

Jdforrester (WMF) (talkcontribs)

> Messages shouldn't need dispute: Messages in en.json should use American English, since en-gb.json and en-ca.json exist for other varieties, while en-us.json doesn't because we use .

I think this anachronism needs to change, but TWN has vetoed it sadly.

Nikerabbit (talkcontribs)

Nobody has actually told me any compelling reason to change how we do messages. I do not consider "language politics" as such to justify having to maintain both en-us and eb-gb separately from en. And I am against showing them an inconsistent/mixed version.

I suggested that we could actually call it American English in language selectors, but that idea didn't get much support on IRC.

Reply to "Preferred spelling"
Mattflaschen (talkcontribs)

Can someone start a Puppet conventions page? Or, it's fine if we just defer to an existing Puppet style guide.

This post was posted by Mattflaschen, but signed as Superm401.

Mattflaschen (talkcontribs)

scfc_de` pointed me to it, and I added a link.

This post was posted by Mattflaschen, but signed as Superm401.

Reply to "Puppet conventions"

The lisp for MW mode in emacs does not work in 23.4.1

2
68.5.79.97 (talkcontribs)

I just pasted the MW mode into .emacs for emacs 23.4.1 from ftp.gnu.org, and it did not load correctly when I tried to open MediaWiki's index.php file. I don't really know lisp, so I can't really debug what happened.

Mattflaschen (talkcontribs)
Reply to "The lisp for MW mode in emacs does not work in 23.4.1"

eg prefix for extension config variables

7
Peachey88 (Flood) (talkcontribs)

How about using eg as prefix for extension configuration variables? Some extensions do it and I have adopted the style for my extensions as well. I find it quite handy with code completion to have all extension globals coming up fast.

This post was posted by Peachey88 (Flood), but signed as Danwe.

Reach Out to the Truth (talkcontribs)

It was in there for three years; see r70755 and comments. That's why you've seen other extensions do it.

Danwe (talkcontribs)

I don't quite understand, so it's no longer best practice to use it for extensions? Why not?

πŸ˜‚ (talkcontribs)

It's not that it's not best practice, it's that it snuck into the coding conventions under the radar, people read it as gospel, and now we've got $eg and ef all over the place when nobody ever really recommended it to begin with.

You can call your config globals $the_most_awsome_variable_in_the_world if you really wanted to, it doesn't matter in a practical sense.

Danwe (talkcontribs)

So what would be the way to get it back into coding conventions in a proper way? Doesn't seem that bad and more consistency in extension development wouldn't harm I guess.

Krinkle (talkcontribs)
Reply to "eg prefix for extension config variables"
Peachey88 (Flood) (talkcontribs)

I've seen some extension use the 'ef' prefix for global function names (presumably standing for 'extension function'). Is this a proper coding convention or some unholy bastardization? Is 'wf' or 'ef' preferred for global function names in extensions?

This post was posted by Peachey88 (Flood), but signed as Kaldari.

Peachey88 (Flood) (talkcontribs)

ef has been used since ancient times, but it's not super common now. In most cases modern style puts hook functions as static methods on a class, leaving few or no raw top-level functions to be so named.

This post was posted by Peachey88 (Flood), but signed as Brion VIBBER.

Reply to "ef prefix"
Peachey88 (Flood) (talkcontribs)

I was wondering if there was any specific reason why the prefix 'wg' is used with global variables. I am looking at starting a project and was wondering if I should just pick a couple letters as a prefix or if there is a system I should be using to choose the letters. Thanks.

This post was posted by Peachey88 (Flood), but signed as Imperator3733.

Peachey88 (Flood) (talkcontribs)
Peachey88 (Flood) (talkcontribs)
Reply to "wg prefix"
Peachey88 (Flood) (talkcontribs)
  • Checking code for a coding standard without automatic checking is very inefficient.
  • There is PHP_CodeSniffer which supports a lot of PHP standards (PEAR, Zend, PHPCS, Squiz, Kohana...)
  • But none of these standards are compatible with MediaWiki coding standards.
  • Do MediaWiki-standard for PHP_CodeSniffer exists? May be anyone know? Π‘an anyone easily make it?
Peachey88 (Flood) (talkcontribs)
Peachey88 (Flood) (talkcontribs)

We have stylize.php which will automagically convert code to proper standard.

This post was posted by Peachey88 (Flood), but signed as Bryan.

Peachey88 (Flood) (talkcontribs)
Peachey88 (Flood) (talkcontribs)
Hashar (talkcontribs)
Reply to "Standards for CodeSniffer"