Difference between revisions of "Getting Your Pull Accepted"

From NSV13
Jump to navigationJump to search
imported>Jordie0608
imported>Jordie0608
Line 67: Line 67:
  
 
Most of the time, Git is pretty smart about merging code files. However, if one PR is merged before yours that edits the same file, your PR will be in a state of conflict and will not be able to be automatically merged via GREEN BUTTON. Because there are more contributors than there are maintainers, most of the time resolving these conflicts falls to the maker of the PR.
 
Most of the time, Git is pretty smart about merging code files. However, if one PR is merged before yours that edits the same file, your PR will be in a state of conflict and will not be able to be automatically merged via GREEN BUTTON. Because there are more contributors than there are maintainers, most of the time resolving these conflicts falls to the maker of the PR.
 +
 +
Usually to fix a merge conflict you will need to pull from upstream on your working branch. This will update all of your code to the most recent revision where it can. Git will then report to you the conflicting files. You will have to manually update the conflicted files with your changes. When you're finished commit your changes.
 +
 +
If it's a map file that's conflicted, remember to start the [[Map Merger]] before you pull from upstream.
  
 
[[Category:Game Resources]]
 
[[Category:Game Resources]]

Revision as of 21:56, 11 October 2014

So you've made a change to the code/sprites and want to make a Pull Request so it can be added to the game!

Before you do this, though, there are 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.

Pull Request Requirements

1. You must follow the coding standards

Please read through these Coding_Standards. The standards will change over time. Hopefully you already read through and made sure your code adhered to them before opening your pull request!

2. Your code must compile.

This is a given. While your pull request will not be closed over this, it will not be accepted 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.

Sometimes Travis is a silly bot and something unrelated to your code causes his compile to fail. If this happens you can force a rebuild by closing and reopening your pull request. Alternatively, you can ask a maintainer to force a rebuild from the Travis page (must be logged in).

If you change an object's path, you must update any maps that have that item placed on it. Travis checks all maps, including Ministation and Metastation. If Travis is failing you after a path change and you don't know why, check the other maps!

3. Do not automatically add FILE_DIR.

A recurring problem 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.

4. 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 granted some leniency, however not all bugfixes are made equal. It's possible to have a change that technically fixes a bug, but does so in a way that's hacky or otherwise undesirable.

5. Make explicit commit logs

Be clear in exactly what each of your commits do. Esoteric or jokey commit logs are hard for future coders to interpret which makes tracing changes harder. Don't make it too long however since an actual description of your changes goes into your pull request's comment.

Take an example of you're adding a new solar panel array to the map.

Bad:

MOAR SOLAR POWAH!

Acceptable:

Adds a new solars above the brig.

6. Clearly state what your pull request does

Don't withhold information about your pull request. 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.

Merge Conflicts

Most of the time, Git is pretty smart about merging code files. However, if one PR is merged before yours that edits the same file, your PR will be in a state of conflict and will not be able to be automatically merged via GREEN BUTTON. Because there are more contributors than there are maintainers, most of the time resolving these conflicts falls to the maker of the PR.

Usually to fix a merge conflict you will need to pull from upstream on your working branch. This will update all of your code to the most recent revision where it can. Git will then report to you the conflicting files. You will have to manually update the conflicted files with your changes. When you're finished commit your changes.

If it's a map file that's conflicted, remember to start the Map Merger before you pull from upstream.