Introduce the revised coding style.

[lioncash]

Opinion time!

I did a general revising of the coding style, since the one on the wiki is quite old. Also this will put it in a file within the repo, which is good because whenever someone makes a pull request it will have a little notification that tells people to make sure that they've read the guidelines (see here for more on that). Having it in the repo will be much easier for potential contributors to see instead of the wiki.

This is the first 'draft' so I strongly encourage people to give me opinions and suggestions on this until we agree upon things.

[Tilka]

Other than the comments: lgtm. Adding some justifications would be helpful to reevaluate these guidelines in the future (and would give people an idea when/why to violate them).

[comex]

I dislike making random local variables / parameters const (assuming you're not referring to const pointers/references). I have been using Rust lately, which requires 'mut' for mutable variables to assist the reader, since most variables need not be; but in Rust there is no syntactic overhead for immutable ones, while in C++, writing const everywhere is very noisy. And there was recently some debate about removing that from Rust...

Either way, the rule should apply to all locals, not just function parameters.

[lioncash]

@comex Reflected it to include locals as well.

@Tilka Reflected as necessary.

(will not force push any more and squash when everything is done. I forgot Github kicks away comments)

[neobrain]

Just saying, we also have the option of just adding a link to https://wiki.dolphin-emu.org/index.php?title=Coding_Style in this file instead. I'm somewhat unhappy with this being basically github-only, when a big part of our developer documentation resides on wiki.dolphin-emu.org.

[galop1n]

HANDLE should be instead something like "using HandleRsc = a std::unique_ptr<HANDLE,HandleDeleter>;" in the first place. RAII should be a strong recommendation, a line of code you do not have to write ( the release/delete ) is a line of code that will not bug

[shuffle2]

I don't really care what is eventually picked, because everyone has varying opinions...but whatever it is NEEDS to be automatically enforceable by a tool. Preferably by VS' and others' built-in solutions for syntax formatting, or at least by an external tool like clang-format or editorconfig which we can package in the dev process.
Note: afaik, express versions of VS don't have plugin support, so we'd need to have some workaround for this scenario if not using the built-in formatter.

[neobrain]

By the way, what's the general opinion on forward declaring things in headers instead of blindly including the declaring header, in order to reduce compilation times? http://stackoverflow.com/questions/9906402/should-one-use-forward-declarations-instead-of-includes-wherever-possible provides a good summary of other reasons.

E.g. struct NativeVertexFormat; instead of #include "VideoCommon/NativeVertexFormat.h".

[delroth]

This is +1 from me if this is worded as an advice/recommendation, not as a
rule. It's a pain to enforce this kind of things - if you want to do so,
then please provide tooling.

(Making Dolphin work with IWYU would be great :) ).

On Fri, Jun 6, 2014 at 6:22 PM, Tony Wasserka notifications@github.com
wrote:

By the way, what's the general opinion on forward declaring things in
headers instead of blindly including the declaring header, in order to
reduce compilation times?
http://stackoverflow.com/questions/9906402/should-one-use-forward-declarations-instead-of-includes-wherever-possible
provides a good summary of other reasons.

E.g. struct NativeVertexFormat; instead of #include
"VideoCommon/NativeVertexFormat.h".

—
Reply to this email directly or view it on GitHub
#410 (comment).

Pierre "delroth" Bourdon delroth@gmail.com
Software Engineer @ Zürich, Switzerland
http://code.delroth.net/

[neobrain]

I'll just give this my LGTM for now. There are still some outstanding minor issues, but I think it's good enough in the current state. IMO issues like using some coding style which is tool-friendly can be addressed another time, since they will likely require a mass-convert of the existing code base anyway.

[lioncash]

Squashed all commits thus far into one.

[neobrain]

There's no rule on whether to use Type *var or Type* var, is there?

[lioncash]

I don't believe there is, though the de-facto accepted way we usually do
PRs is with the asterisk/ampersand against the type.

I can add this to the style if needed.
On Jul 4, 2014 8:31 AM, "Tony Wasserka" notifications@github.com wrote:

There's no rule on whether to use Type var or Type var, is there?

—
Reply to this email directly or view it on GitHub
#410 (comment).

[neobrain]

Yeah, I would say this definitely belongs in the coding style.

[neobrain]

Another thing that was discussed on IRC was how to define type aliases. Do we want the more traditional typedef or do we use the C++11 addition using? (the latter in particular supports template aliases, with anything else probably being personal preference and me favoring using)

[lioncash]

Hmm... I think both should be allowed, since they both do the same thing
for the most part (with the exception of template aliases with using,
like you said)
On Jul 4, 2014 9:02 AM, "Tony Wasserka" notifications@github.com wrote:

Another thing that was discussed on IRC was how to define type aliases. Do
we want the more traditional typedef or do we use the C++11 addition using?
(the latter in particular supports template aliases, with anything else
probably being personal preference and me favoring using)

—
Reply to this email directly or view it on GitHub
#410 (comment).

[neobrain]

Yeah, there's probably not much of a point in enforcing usage of a single one of them.

[lioncash]

@neobrain Added formatting for references and pointers (specifically in General)

[lioncash]

So is this OK to merge now?

[delroth]

@lioncash: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.

---

That's the last comment I have. Thanks a lot for spending your time on this! Have you checked that Contributing.md works for GitHub? (I thought it had to be named CONTRIBUTING.md in uppercase, but I can't remember testing the non-uppercase variant).

@dolphin-emu-bot allowmerge