Difference between revisions of "Getting Your Pull Accepted"

From NSV13
Jump to navigationJump to search
imported>Rockdtben
imported>Rockdtben
Line 7: Line 7:
 
== Code Style Requirements ==
 
== Code Style Requirements ==
  
=== 1. Your code MUST compile. ===
+
=== Your code MUST compile. ===
 
This is a given.  While we will not close your pull request over this, we will not accept it until it does compile cleanly.  The [https://travis-ci.org/tgstation/-tg-station Travis] bot will check for this automatically, but you should check before you even commit.
 
This is a given.  While we will not close your pull request over this, we will not accept it until it does compile cleanly.  The [https://travis-ci.org/tgstation/-tg-station Travis] bot will check for this automatically, but you should check before you even commit.
  
 
Warnings should also be cleared, but are not a requirement to fix.
 
Warnings should also be cleared, but are not a requirement to fix.
  
=== 2. Variables, functions, and objects must be clearly named. ===
+
=== You MUST follow the coding standards ===
Pulls adding variables that are nonsensical or overly-abbreviated will not be accepted.  Write your code as though you wanted a bunch of drunken idiots to understand it.
+
Please read through these. The standards will change over time. [Coding_Standards]
 
 
'''Bad:'''
 
<pre>
 
var/a
 
var/lol
 
var/derp
 
var/herp
 
proc/lcb(var/p_1,var/p_2)
 
</pre>
 
 
 
'''Acceptable:'''
 
<pre>
 
var/count
 
var/html
 
var/damage_received
 
proc/localCallback(var/caller,var/params_to_pass)
 
</pre>
 
 
 
{{note|Using <code>i</code> in a <code>for</code> loop <em>is</em> acceptable, as it's a common practice.}}
 
 
 
=== 3. Absolute Pathing is Essential ===
 
When creating a new atom or proc, please use absolute pathing, which makes it far easier to search for things and breaks up files a bit.
 
 
 
'''NO:'''
 
<pre>
 
obj
 
  item
 
      flashlight
 
          var
 
              on = 0
 
          proc
 
              turn_on()
 
</pre>
 
 
 
'''Acceptable:'''
 
<pre>
 
/obj/item/flashlight
 
    var/on = 0
 
/obj/item/flashlight/proc/turn_on()
 
    ...
 
</pre>
 
 
 
{{note|Due to reagents being a horrible fucking mess, it is currently permissible to ignore this rule when adding reagents.  Please try to make them readable, however.}}
 
  
 
=== 4. Do not automatically add FILE_DIR. ===
 
=== 4. Do not automatically add FILE_DIR. ===
Line 67: Line 24:
 
To fix this problem:
 
To fix this problem:
 
# Open up DreamMaker
 
# Open up DreamMaker
# Build > Preferences for baystation12.dme...
+
# Build > Preferences for tgstation.dme...
 
# '''Uncheck''' "Automatically set FILE_DIR for sub-directories"
 
# '''Uncheck''' "Automatically set FILE_DIR for sub-directories"
 
# Check compile.
 
# Check compile.

Revision as of 07:32, 3 June 2014

Work in progress...

So you've made a change to the code/sprites and want to make a Pull Request so we can add it to the game.

Before you do this, though, we have some requirements so your code doesn't make SS13's infamous spaghetti code problems even worse. Make sure your code fits these requirements or your pull will not be accepted. You may also want to read through the suggestions section.

Code Style Requirements

Your code MUST compile.

This is a given. While we will not close your pull request over this, we will not accept it until it does compile cleanly. The Travis bot will check for this automatically, but you should check before you even commit.

Warnings should also be cleared, but are not a requirement to fix.

You MUST follow the coding standards

Please read through these. The standards will change over time. [Coding_Standards]

4. Do not automatically add FILE_DIR.

A recurring problem that we encounter is people adding commits that needlessly spam changes to the DME due to having "Automatically add FILE_DIR" set in their project settings. You'll know if you have this problem if you see this in your commit (as seen through github):

Fucking FILE DIR.png

PRs that add things to FILE_DIR will be rejected.

To fix this problem:

  1. Open up DreamMaker
  2. Build > Preferences for tgstation.dme...
  3. Uncheck "Automatically set FILE_DIR for sub-directories"
  4. Check compile.
  5. Close DreamMaker and commit again.

5. Pull Requests Must Be Atomic

Pull requests must add, remove, or change only ONE feature or related group of features. Do not "bundle" together a bunch of things, as each change may be controversial and hold up everything else. In addition, it's simply neater.

Bugfix-only PRs are exempt from this rule. Everyone loves bugfixes.


6. Follow The Stylesheet.

Changing formatting with \red or other tags is not allowed. You must use span classes, which are defined in interface/stylesheet.dm.


7. Clearly State What Your Pull Request Does, All Of It

Please do NOT withhold information about your pull request from us. This will get your pull request delayed and if it happens too often your pull request will be closed.

Suppose you fixed bug #1234 and changed the name and description of an item as a gag.

Bad:

In this PR I fixed bug #1234.

Acceptable:

In this PR I fixed bug #1234 and also modified the baton's name and description.