Manual talk:Coding conventions

From MediaWiki.org
Jump to: navigation, search
Start a new discussion

Contents

Thread titleRepliesLast modified
Dead Links017:46, 8 July 2014
Puppet conventions122:00, 26 February 2014
Trailing whitespace700:31, 5 January 2014
Release notes and bug fixes514:50, 1 April 2013
The lisp for MW mode in emacs does not work in 23.4.1100:40, 6 December 2012
Standards for CodeSniffer507:47, 11 April 2012
Dedicated pages per programming language018:18, 20 December 2011
Documentation, even if this is MediaWiki801:38, 6 December 2011
Globals and Top-Level Functions123:40, 5 December 2011
eg prefix for extension config variables622:12, 5 December 2011
ef prefix107:16, 5 December 2011
wg prefix207:15, 5 December 2011
Deprecate private variables and methods?307:11, 5 December 2011
What about carriage returns between sections of code?107:09, 5 December 2011

Dead Links

The Section http://www.mediawiki.org/wiki/Manual:Coding_conventions#eclipse_style contains the links http://pde.dubture.com/ and http://wikidemo.formulasearchengine.com/images/MediaWiki.xml which are both dead. Where can the correct URLs be aquired (especially for the second link, to the Coding Style file for Eclipse)?

193.158.165.25017:46, 8 July 2014

Puppet conventions

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

Superm401 - Talk21:16, 26 February 2014

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

Superm401 - Talk22:00, 26 February 2014
 

Trailing whitespace

Why do we want to get rid of that section on trailing whitespace? Some newbies might not realize their text editor can be configured to get rid of it automatically.

Leucosticte (talk)05:42, 30 December 2013

I also think "Don't add trailing whitespace" is a good coding guideline.

Anomie (talk)14:36, 30 December 2013

The intention of the policy is to make it easier on people who are using text editors like Notepad++ (which I use extensively), where pressing the End key on the keyboard goes all the way to the end of the line, not to the end of last visible text. And considering removing extra whitespace is a trivial operation on almost all text editors (even Notepad++), it is an easy fix to a sometimes annoying bug.

Additionally, there is absolutely no way I will agree with removing this whitespace guideline unless there is a way to remove the goddamn red highlighting that appears all over diffs. Although even if there is a way, like I mentioned above, I don't see why we should be removing this guideline.

Parent5446 (talk)15:44, 30 December 2013
 

I think the more important question is why was it removed almost randomly without any notice or discussion?

Parent5446 (talk)15:41, 30 December 2013
 

The convention was added in this edit. It was expanded a bit in subsequent edits by Nemo and others.

The current language is pretty stupid, in my opinion. I think it gives the impression that a trailing newline in a file is a bad thing, which is obviously not true, and it provides an overbearing "don't do this" without providing any rationale for why this is problematic.

MZMcBride (talk)16:14, 4 January 2014

It seems there's a whole separate section about trailing newlines (as opposed to trailing whitespace). This is... a bit much. We can probably make these coding conventions less onerous.

MZMcBride (talk)16:17, 4 January 2014

Check out my edit. How does that wording sound?

Parent5446 (talk)20:20, 4 January 2014

Better. :-)

But I still think it may make sense to combine the "newlines" section with the "trailing whitespace" section, as I think it could add clarity.

MZMcBride (talk)00:31, 5 January 2014
 
 
 
 

The guideline says you must add to release notes when you fix a bug, but in practice this is almost never respected, especially because with git it makes merge conflicts almost certain. Automatic updates of release notes or other variants (like herds of slaves doing it in the devs' stead) are currently just dreams.

To the few who care and watch this page: remember that release notes become even more important if you remove the bug numbers from the first line of commit messages.

Nemo14:38, 10 March 2013

See also Thread "Git release notes merge conflicts" on wikitech-l (April 2012). Several ideas for a format and automated tool have been proposed there.

However the discomfort and some people neglecting to include release notes does not justify anything. Until and unless we have a different tool, one is expected to include release notes. You'll just have to deal with it, whether you like it or not. If you don't then the change doesn't get merged (it is trivial to amend a patch and add release notes).

You claim of it being "almost never respected" is simply untrue, there are only a small number of changes from relatively few people that don't provide release notes when they should. Most people do when they should.

By the way, I don't see how Talk:Gerrit/Commit message guidelines#Bug number in commit messages is relevant? If anything, moving them from the subject to the body keep the raw git-log easier to scan. More on that over on that talk page.

Krinkle (talk)20:20, 12 March 2013

I agree that release notes are important, but your "small number of changes" is just wishful thinking.

As of now there are over 350 bugs marked fixed since 1.20 was branched, and only 80 mentioned in RELEASE-NOTES-1.21 (which seems to have about 150 bullets in total)... of course it's just approximations but it seems clear that only a minority of bug fixes follow the convention.

Nemo20:46, 12 March 2013

I mentioned this topic on the thread Who is responsible for communicating changes in MediaWiki to WMF sites?, I think it bears some relevance on that matter too (but it's a separate one).

Nemo12:57, 23 March 2013

Historically we did not (and presumably still don't) include release notes for a bug fix that was introduced in the same release as it was fixed. Some of these bugs may be that sort of situation.

Bawolff (talk)23:43, 31 March 2013

This should be noted in the conventions, anyway it would leave us with at least 190 bugs fixed as of now (those filed before the branching) plus 27 enhancements (requested after the branching), and 188 reports that may need or not be mentioned.

Nemo14:50, 1 April 2013
 
 
 
 
 

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

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.

68.5.79.9700:35, 20 February 2012

I made a couple fixes that get it working for me (Emacs 23.4.1). They are pending review.

Superm401 - Talk00:40, 6 December 2012
 

Standards for CodeSniffer

  • 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?
—The preceding unsigned comment was added by an unknown user on a unknown date.07:13, 5 December 2011

Yeah, I second this. The staff is working on a phpUnderControl instance, so I'm sure they will get around to this eventually.

--Cneubauer 17:58, 17 September 2010 (UTC)07:13, 5 December 2011
 

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

-- Bryan (talk|commons) 17:50, 28 October 2010 (UTC)07:14, 5 December 2011

Where is that? I don't see it in the maintenance directory.

--Cneubauer 19:09, 29 November 2010 (UTC)07:14, 5 December 2011

It's here.

Max Semenik 19:10, 29 November 2010 (UTC)07:14, 5 December 2011
 
 

If anyone is interested, I have created a very basic MediaWiki coding style and it is now hosted by the Wikimedia Fundation as mediawiki/tools/codesniffer.git

Gerrit: https://gerrit.wikimedia.org/r/#q,project:mediawiki/tools/codesniffer,n,z

To clone it:

git clone https://gerrit.wikimedia.org/r/p/mediawiki/tools/codesniffer

Antoine "hashar" Musso (talk)07:47, 11 April 2012
 

Dedicated pages per programming language

I've generalized this page to be a code conventions portal. Branching off specific languages to other dedicated pages. The "do as PHP unless stated otherwise" is getting old because JavaScript, for instance, is simply very different than PHP. And although the end result in syntax may be similar at times, the reasoning behind is very different. Therefor it's better to describe it in the right context with examples that make sense.

See also the Restructure MediaWiki.org-thread on Project:Current issues.

Krinkle18:18, 20 December 2011

Documentation, even if this is MediaWiki

I propose merging in these guidelines to improve the quality of code documentation and increase code readability.

-- Jeroen De Dauw 11:58, 21 July 2010 (UTC)07:11, 5 December 2011

Sounds like a good idea.

Tisane 16:18, 21 July 2010 (UTC)07:12, 5 December 2011

Sounds nice, except for the part about using @since...do we really want that?

--Jack Phoenix (Contact) 18:06, 21 July 2010 (UTC)07:12, 5 December 2011

I don't see any reason not to have it, and a lot of reason to do have it. I figure it's not really possible to add it to all current code accuratly, but I'd be very nice if people just added it when writing new code.

--Jeroen De Dauw 21:08, 25 July 2010 (UTC)07:12, 5 December 2011

I like almost all of your proposals, just a bit skeptical with the @since. I have noticed, you have been using this in your extensions but even if you tag something as @since 0.1 and in 0.2 the whole function changes and gets new arguments, I am not sure you would update it to @since 0.2 or add a note. One way would be using multiple @since in this case and adding a note behind that how it was different back then. Anyway, I have missed this many times in MW core, would be so much easier to keep backwards compatibility if this were documented properly! Especially for public functions but also for important constants and globals.

Danwe01:22, 6 December 2011
 
 
 

No further objections to me merging this stuff in?

-- Jeroen De Dauw 05:17, 9 August 2010 (UTC)07:12, 5 December 2011
 

Go go gadget wikimerger!

Krinkle23:37, 5 December 2011

This thread is over a year old...how did so many old threads get bumped?

^demon00:43, 6 December 2011

Because of the conversion to LQT :-)

Helder01:38, 6 December 2011
 
 
 

Globals and Top-Level Functions

Since Manual:Global variables discourages the use of globals, this Manual:Coding conventions should encourage the use of static class members and functions in lieu of globals and top-level functions. Instead of

$wgVersion;
$wgTitle;
 
function wfFuncname() { ... }

it could encourage

class SomeClass {
    public static $version;
    public static $title;
 
    public static function funcname() { ... }
}
Sledged (talk) 19:23, 5 March 2008 (UTC)07:08, 5 December 2011

As far as I can see the current coding conventions doesn't encourage use of global variabels and functions, it merely documents how to name them if you decide to create a global function or variable.

In the same section it also says how class method names and field names should be named.

Manual:Coding_conventions#PHP_Naming.

Krinkle23:40, 5 December 2011
 

eg prefix for extension config variables

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.

--Danwe 04:28, 3 December 2011 (UTC)07:17, 5 December 2011

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

Reach Out to the Truth15:47, 5 December 2011

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

Danwe16:00, 5 December 2011

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.

^demon16:21, 5 December 2011

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.

Danwe17:51, 5 December 2011

Extensions should just use $wg, like core.

^demon22:04, 5 December 2011
 
 
 
 
 

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?

Kaldari 23:51, 27 April 2011 (UTC)07:16, 5 December 2011

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.

--brion 23:57, 27 April 2011 (UTC)07:16, 5 December 2011
 

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.

--Imperator3733 06:19, 7 December 2010 (UTC)07:15, 5 December 2011

It stands for "Wikipedia Global". I'm not sure that qualifies as a system. :-)

Emufarmers(T|C) 02:00, 8 December 2010 (UTC)07:15, 5 December 2011

Ok, thanks. Not sure why I didn't see that article before.

--Imperator3733 07:49, 8 December 2010 (UTC)07:15, 5 December 2011
 
 

Deprecate private variables and methods?

Shall we deprecate private variables and methods? They seem evil, since they make it more cumbersome to extend classes.

Tisane 08:08, 8 July 2010 (UTC)07:10, 5 December 2011

We're actually trying to use them in new code because they help in separating abstractions and give us better better control over proper interfaces to access data. You can't force people to uses accessor methods if they still can access class variables directly. Unfortunately, due to PHP's low entry requirements, many PHP programmers know nothing about proper OOP/OOD and produce such opuses instead.

Max Semenik 10:02, 8 July 2010 (UTC)07:10, 5 December 2011

Protected seems preferable, for extensibility purposes.

Tisane 16:15, 8 July 2010 (UTC)07:10, 5 December 2011

It depends. Public, private and protected all have their places. Some times you want something private because you /don't/ want subclasses to be able to change the implementation.

^demon07:11, 5 December 2011
 
 
 

What about carriage returns between sections of code?

When is it good to include blank lines? I usually include a blank line between functions, classes, etc. But is it also good style to include blank lines between, say, major code sections within a function? That leads to the question of what counts as a major section of code within a function; I find it to be a pretty arbitrary/subjective decision, and ultimately having a lot of blank lines just hinders the reader from seeing very much of the code without scrolling.

Tisane 14:52, 3 June 2010 (UTC)07:09, 5 December 2011

Pretty much as you say: they're good, but should be used sparingly; it's much easier to say "apply common sense" than to try and legislate for it.

Happymelon 16:52, 3 June 2010 (UTC)07:09, 5 December 2011