Expectations for newbie dev contributors?

[decadentPilgrim]

Hello,

I took a quick skim on the forum and didn't see a page pertaining to this so I'll try starting this thread. I'm interested in knowing how folks working on WZ 2100 project codebase prefer new members to contribute, so as not to create unnecessary work, frustrations or step on any folks toes. As such, I'd be interested to know:

• Are there code review guidelines I should read up on (ie- norms on naming conventions, styling, pointer use, etc?)
• Is there a solid way for folks to get a trusty windows build going, which I should adopt?
• Any per-checkin build steps or unit tests which should be run through for sanity checking of code changes? I don't have a mac on my end- not a problem, I hope.
• Is there some kind of peer code review/signoff process for code changes to the trunk?

If this information is already conveniently posted somewhere, a link to those details would be appreciated.


Right now, I'm pulling the SVN view of the project and will start looking through the code, buglist and forums to get an idea of how things have been going.

Thanks for the help!

[Safety0ff]

Some of that is covered here: http://developer.wz2100.net/wiki/Coding

[decadentPilgrim]

Thanks, yeah that seems to cover most of the items I'm looking for at this stage...

[Zarel]

Most importantly: If you want to contribute, write a patch, diff a patchfile from it, create a ticket for it, and if you did anything wrong, we can tell you then.

You can extrapolate coding guidelines from the way the code currently is. Some if it's contradictory, since there's some old code, so a few things to keep in mind:

- We prefer uint32_t to UDWORD, although int is usually fine if you're not dealing with something like netcode or savefiles.
- Hungarian notation (psSubject instead of subject) is unnecessary.
- We prefer if (boolean) to if(boolean)

Some other things to keep in mind:

- Signed is nearly always better than unsigned.
- Fixed point is nearly always better than floating point.

[decadentPilgrim]

Thanks Guys,

At this point, I've got svn view of trunk, working game build with VS 2008, running under the debugger and getting acquainted with the code docs. So far, so good.


A couple follow up questions:
+With regards to guidance of "Patches as a rule go into the patch tracker" - I didn't see info for it when searching for "patch tracker". Is this just referring to a ticket filed as type "patch" under the ticket/work tracker(would align to what Zarel is suggesting), or something else?
from http://developer.wz2100.net/wiki/CommitGuidelines

+ I noticed that the end of the coding style guidelines makes mention of a couple pretty printing tools: indent and astyle, whereas the commit guidelines recommend conservative scope on stylistic changes(quite sane in context of established codebase). As a quick test, I ran the pretty printers with the style guidelines, which as expected produced huge churn of delta's, primarily wrt indentation. As there wasn't much commentary on how the pretty printers fit into a typical dev's workflow, I have a couple of questions - was the mention about the pretty printers conceived as a local sanity test for highlighting any styling gaffes when preparing patches? Is the pretty printer use a normal part of most contributors workflows, or an aspirational item?

Assuming no objections, I'll make clarifying changes to pertinent wiki pages based on replies.

[cybersphinx]

Some of the stuff in the wiki is quite old, I've changed it to just "trac" now.

For coding style, if you change things, only change those lines that are changed, so diffs are easy to read, new code could be run through a beautifier if needed (old code as well, if it's necessary, but as a separate commit then that only changes the style).

[decadentPilgrim]

Got it - that stuff makes sense.

On a preliminary skim of the bugs, I didn't see any that immediately jumped out at me as small scale codebase chores of low risk gruntwork. If anyone has any routine sets of cleanups/refactorings you'd have wanted to see done anyway, let me know - that may be a decent way for me to start getting more acquainted with the code.


On the code documentation page, there is an square bracketed mention of Doxygen generated docs - Should this be a working hyperlink on the site? If not, perhaps a change to just point folks to installing & running doxygen themselves on their local build would be suitable:
http://developer.wz2100.net/wiki/Code_Documentation


Finally, I noticed that a lot of C style commented declarations under trunk/src have been "doxygenized" in past changes, but a fair bit still remains in old style. While I skim over the code, would it be reasonable if I prep purely non-functional change patches covering doc conversion to use doxygen? I'm thinking on a per header+implementation file basis? This would give me a chance to get practice with doxygen's syntax.

Thanks again folks!