Getting Your Pull Accepted: различия между версиями

Материал из MassMeta
Перейти к навигации Перейти к поиску
ВизуальныйВики-текст
imported>Rockdtben
(Created page with "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 d...")
 
imported>SpaceManiac
м (Remove "Game Resources" category)
 
(не показаны 43 промежуточные версии 9 участников)
Строка 1: Строка 1:
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.
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, 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.
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.


== Code Style Requirements ==
= Pull Request Requirements =


=== 1. Your code MUST compile. ===
=== 1. You must follow the coding standards ===
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 [http://jenkins-ci.org Jenkins] bot will check for this automatically, but you should check before you even commit.
Please read through these [https://github.com/tgstation/tgstation/blob/master/.github/CONTRIBUTING.md 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!


Note that sometimes a maintainer will screw up and commit bad code to Bleeding-Edge, which may affect your pull.  When this happens, we will have Jenkins re-evaluate your pull after we've cleared the problem (and appropriately yelled at however caused the mess).
=== 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 [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.


Warnings should also be cleared, but are not a requirement to fix.
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).


=== 2. Variables, functions, and objects must be clearly named. ===
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!
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.


'''Bad:'''
=== 3. Do not automatically add FILE_DIR. ===
<pre>
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):
var/a
 
var/lol
[[Image:Fucking_FILE_DIR.png|600px|center]]
var/derp
 
var/herp
PRs that add things to FILE_DIR will be rejected.
proc/lcb(var/p_1,var/p_2)
 
</pre>
To fix this problem:
# Open up DreamMaker
# Build > Preferences for tgstation.dme...
# '''Uncheck''' "Automatically set FILE_DIR for sub-directories"
# Check compile.
# 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. Ideally the first line should be a short summary, then you have a more fleshed out commentary below that.


'''Acceptable:'''
Make sure you also add a changelog entry if you're making a big player facing change.
<pre>
The guide to adding changelogs can be found in the [[Guide_to_Changelogs]]
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.}}
=== 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.


=== 3. Absolute Pathing is Essential ===
Suppose you fixed bug #1234 and changed the name and description of an item as a gag.
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:'''
'''Bad:'''
<pre>
<pre>
obj
In this PR I fixed bug #1234.
  item
      flashlight
          var
              on = 0
          proc
              turn_on()
</pre>
</pre>


'''Acceptable:'''
'''Acceptable:'''
<pre>
<pre>
/obj/item/flashlight
In this PR I fixed bug #1234 and also modified the baton's name and description.
    var/on = 0
/obj/item/flashlight/proc/turn_on()
    ...
</pre>
</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.}}
== How to Deal with 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.


=== 4. Do not automatically add FILE_DIR. ===
An easy way to fix merge conflicts is to make a new branch (you should especially do this if your PR is just a small one):
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):
# Switch to your master branch: Right-click your working folder, go '''TortoiseGit''' --> '''Git Switch/Checkout...'''
# Look at the '''Branch'''-list and select '''master'''
# Close the window that pops up (assuming it changed successfully)
# Right-click your working folder, go '''TortoiseGit''' --> '''Git Pull...'''
# On the top, from the '''Remote'''-list, select '''upstream''' and from the '''Remote Branch''' -list select '''master'''.
# Close the window that pops up (assuming it updated successfully)
# Make the same exact changes to the code which you have on your PR
# Commit and MAKE A NEW BRANCH
# Push your new branch to your PR branch, and tick '''Force: unknown changes'''


[[Image:Fucking_FILE_DIR.png|800px|center]]


PRs that add things to FILE_DIR will be rejected.
If it's a map file that's conflicted, remember to start the [[Map Merger]] before you pull from upstream.


To fix this problem:
# Open up DreamMaker
# Build > Preferences for baystation12.dme...
# '''Uncheck''' "Automatically set FILE_DIR for sub-directories"
# Check compile.
# 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.
{{Contribution guides}}

Текущая версия от 06:40, 4 декабря 2020

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. Ideally the first line should be a short summary, then you have a more fleshed out commentary below that.

Make sure you also add a changelog entry if you're making a big player facing change. The guide to adding changelogs can be found in the Guide_to_Changelogs

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.

How to Deal with 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.

An easy way to fix merge conflicts is to make a new branch (you should especially do this if your PR is just a small one):

  1. Switch to your master branch: Right-click your working folder, go TortoiseGit --> Git Switch/Checkout...
  2. Look at the Branch-list and select master
  3. Close the window that pops up (assuming it changed successfully)
  4. Right-click your working folder, go TortoiseGit --> Git Pull...
  5. On the top, from the Remote-list, select upstream and from the Remote Branch -list select master.
  6. Close the window that pops up (assuming it updated successfully)
  7. Make the same exact changes to the code which you have on your PR
  8. Commit and MAKE A NEW BRANCH
  9. Push your new branch to your PR branch, and tick Force: unknown changes


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


Hosting Hosting a server • Setting up the database • Working with /tg/station as an upstream repository
Contributing Guide to contributing to the game • Setting up git • Downloading the source code • Reporting issues • Changelogs
Coding Understanding SS13 code • SS13 for experienced programmers • Code docs • Coding standards • Getting Your Pull Accepted • Binary flags‎ • Text Formatting • MySQL
Mapping Guide to mapping • Map merger • Guide to door access
Spriting Guide to spriting • Resolving icon conflicts
Wiki Guide to contributing to the wiki • Wikicode • Autowiki
Источник — https://massmeta.ru/index.php?title=Getting_Your_Pull_Accepted&oldid=11526