Difference between revisions of "DeveloperGuide"

From The Battle for Wesnoth Wiki
(Commits)
m
(14 intermediate revisions by 7 users not shown)
Line 1: Line 1:
 +
This page describes guidelines developers with push access to the Wesnoth repository should follow. Most items apply to prospective contributors as well, in addition to the [[PatchSubmissionGuidelines|patch submission guidelines]].
 +
 +
__TOC__
 +
 
== General ==
 
== General ==
  
* [[WesnothRepository#Push_access|Set up developer access to the repository.]]
+
{{NewDevsGoHereSidebox}}
<!-- FIXME: DEAD LIST * Subscribe to the commit mailing list: https://mail.gna.org/listinfo/wesnoth-commits or get the list moderator to approve commit messages from you otherwise -->
+
 
* Be on the #wesnoth-dev IRC channel on freenode, and coordinate with other developers. Bots report commits and other developers may ask you questions or provide feedback.
+
* [[WesnothRepository#Push_access|Set up developer access to the repository]], if applicable.
 +
* Subscribe to the [https://mailman.wesnoth.org/listinfo/dev wesnoth-dev mailing list], wherein major announcements are made and project-changing discussions take place.
 +
* Participate in the #wesnoth-dev IRC channel on irc.freenode.net to coordinate with other developers. Bots report commits and regression testing status, and other developers may ask you questions or provide feedback.
  
 
== Commits ==
 
== Commits ==
Line 10: Line 16:
 
* All <b>unit tests</b> should pass after commit -- Travis will also test this. To run the C++ unit tests on your own machine, compile the "test" executable and run it. To run the WML unit tests, run <code>run_wml_tests -u</code> in the checkout root. If a test times out spuriously on Travis, you can restart the build by  
 
* All <b>unit tests</b> should pass after commit -- Travis will also test this. To run the C++ unit tests on your own machine, compile the "test" executable and run it. To run the WML unit tests, run <code>run_wml_tests -u</code> in the checkout root. If a test times out spuriously on Travis, you can restart the build by  
 
*# Clicking on the link posted to IRC by the Travis bot:
 
*# Clicking on the link posted to IRC by the Travis bot:
*#: 20141009 20:40:27-!- travis-ci [~travis-ci@ec2-184-73-55-78.compute-1.amazonaws.com] has joined #wesnoth-dev
+
*#: 20180608 07:25:20-!- travis-ci [~travis-ci@ec2-54-204-112-72.compute-1.amazonaws.com] has joined #wesnoth-dev
*#: 20141009 20:40:27< travis-ci> wesnoth/wesnoth#4154 (master - c91604a : Ignacio R. Morelle): The build was broken.
+
*#: 20180608 07:25:21< travis-ci> wesnoth/wesnoth#18505 (1.14 - 8722221 : Charles Dang): The build was broken.
*#: 20141009 20:40:27< travis-ci> Build details : http://travis-ci.org/wesnoth/wesnoth/builds/37536345
+
*#: 20180608 07:25:21< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/389584322
*#: 20141009 20:40:27-!- travis-ci [~travis-ci@ec2-184-73-55-78.compute-1.amazonaws.com] has left #wesnoth-dev []
+
*#: 20180608 07:25:21-!- travis-ci [~travis-ci@ec2-54-204-112-72.compute-1.amazonaws.com] has left #wesnoth-dev []
 
*# Clicking on the [http://i.imgur.com/mxwVKJP.png restart button] on the Travis website.
 
*# Clicking on the [http://i.imgur.com/mxwVKJP.png restart button] on the Travis website.
 
* Do <b>not</b> disable or ignore unit tests without a very good reason!
 
* Do <b>not</b> disable or ignore unit tests without a very good reason!
 
* A few small commits are better than a single large commit (which is hard to review), so when possible split it in working parts with info about where you are going
 
* A few small commits are better than a single large commit (which is hard to review), so when possible split it in working parts with info about where you are going
* Always review your changes, both before committing locally and before pushing upstream (see [[WesnothRepository#Reviewing your changes]]).
+
* <b>Always</b> review your changes, both before committing locally and before pushing upstream (see [[WesnothRepository#Reviewing your changes]]).
 +
* <b>Never</b> use the "force" option when pushing to the upstream Wesnoth repositories (see [[WesnothRepository#Force-pushing_policy]]).
  
=== Changelogs and release notes ===
+
=== Changelog and release notes ===
  
We provide two separate changelogs at the root of the Wesnoth distribution and Git checkouts. The <b>players_changelog</b> file only contains entries for changes deemed relevant for players (not content creators), while the main <b>changelog</b> contains all changes, except for those that are only relevant to mainline developers and which are not expected to impact players or creators in any way.
+
We provide a <b>changelog</b> at the root of the Wesnoth distribution and Git checkouts. It contains all changes, except for those that are only relevant to mainline developers and which are not expected to impact players or creators in any way.
  
It is your responsibility to update both changelogs as you see fit for every commit. For large changes spread over long series of commits, you will probably prefer to commit your changelog additions in a separate last commit to avoid or mitigate conflicts when merging upstream.
+
It is your responsibility to update the changelog as you see fit for every commit. For large changes spread over long series of commits, you will probably prefer to commit your changelog additions in a separate last commit to avoid or mitigate conflicts when merging upstream.
  
 
Important changes that might be expected to inconvenience or confuse players (including those building from source) or content creators, major bug fixes, and noteworthy feature additions for a future release should be mentioned and explained in <b>RELEASE_NOTES</b> as well, so that the release team can include this information in official announcements. See the contents of the file for instructions.
 
Important changes that might be expected to inconvenience or confuse players (including those building from source) or content creators, major bug fixes, and noteworthy feature additions for a future release should be mentioned and explained in <b>RELEASE_NOTES</b> as well, so that the release team can include this information in official announcements. See the contents of the file for instructions.
Line 33: Line 40:
 
For Wesnoth in particular, the general consensus is that contributors should adhere to the following rules:
 
For Wesnoth in particular, the general consensus is that contributors should adhere to the following rules:
  
* Every commit message should begin with a self-contained <b>summary/subject line</b> no more than 72 characters long. This is the first (and often the only) line someone using browsing tools will see.
+
* Every commit message should begin with a self-contained <b>summary/subject line</b> no more than 72 characters long. This is the first (and often the only) line someone using browsing tools will see. Like with email subjects, this line should not have a trailing stop.
 
* If your message needs more than a leading summary line, separate it from the rest with a blank line.
 
* If your message needs more than a leading summary line, separate it from the rest with a blank line.
 
* Subsequent lines should also be no more than 72 characters long. Use blank lines to separate paragraphs and list items.
 
* Subsequent lines should also be no more than 72 characters long. Use blank lines to separate paragraphs and list items.
 +
* When working on a mainline campaign/scenario, start the commit message with its acronym. For example <code>HttT: </code> when working on HttT in general and <code>HttT S03: </code> if the commit only applies to the third scenario of HttT (see below).
 +
* Likewise, if you work on the code of a specific component, you may want to use it as a prefix. That is e.g., <code>gui2: </code> or <code>wesnothd: </code>.
 
* If you are performing branch merges, Git may include a list of conflicted paths in the commit message template. Edit this out; it ceases to be interesting after the conflicts have been resolved.
 
* If you are performing branch merges, Git may include a list of conflicted paths in the commit message template. Edit this out; it ceases to be interesting after the conflicts have been resolved.
 
* If you need to mention commit SHA1 hashes (e.g. in revert messages), make sure to use hashes from the upstream repository that are unlikely to change due to a future local or remote merge or rebase operation.
 
* If you need to mention commit SHA1 hashes (e.g. in revert messages), make sure to use hashes from the upstream repository that are unlikely to change due to a future local or remote merge or rebase operation.
Line 42: Line 51:
  
 
* Include "bug #1234" somewhere in your commit message if it addresses a bug or feature request from the Wesnoth bug tracker.
 
* Include "bug #1234" somewhere in your commit message if it addresses a bug or feature request from the Wesnoth bug tracker.
* Use the project's [[WesnothAcronyms|standard abbreviations]] for campaigns and gameplay concepts, like HttT for <cite>Heir to the Throne</cite>, ZoC for "Zone of Control", etc. If referring to a particular campaign or scenario in your commit subject, you should use this convention: "HttT S1: Made Galdrad less suicidal"
+
* Use the project's [[Glossary#Wesnothian_Acronyms|standard abbreviations]] for campaigns and gameplay concepts, like HttT for <cite>Heir to the Throne</cite>, ZoC for "Zone of Control", etc. If referring to a particular campaign or scenario in your commit subject, you should use this convention: "HttT S01: Made Galdrad less suicidal"
 
 
== Bugs management ==
 
* Change status of fixed bugs to "Fixed" when committed
 
* Change status of fixed bugs to "Closed" when released, see [[ReportingBugs#Bug_protocol]] for details
 
* Check if there is new bugs relevant to your code and if any, assign them to you.
 
  
 
== Dependencies ==
 
== Dependencies ==
* Any change in dependencies to build the project must be discussed on the mailing list (dev-talk).
+
* Any changes to Wesnoth's build and run-time dependencies must be discussed on the developers mailing list first.
* Update SConstruct (and ideally cmake recipe also) to check for the dependency.
+
* Update the CMake and SCons recipes accordingly.
* Note any change (added or removed deps, or increased version requirements) in the RELEASE_NOTES file. These will be announced in the forum announcement and in the emails to packagers.
+
* Changes must be mentioned in the RELEASE_NOTES file, so they can be included in the forum announcement for the next release, and in the emails to packagers.
  
== Tips ==
+
== Bugs management ==
 
+
* Change Status field of fixed bugs to "Fixed" after pushing upstream
* If you build with scons, use clang rather than gcc, as it is 2-3x faster to compile wesnoth.
+
* Change Open/Closed field of fixed bugs to "Closed" after a release including the relevant commits has been published. See [[ReportingBugs#Bug_protocol]] for details.
Set your choice permanently with
+
* Check regularly if there are new bugs relevant to your code and, if any, assign them to you.
 
 
scons cxxtool=clang++
 
  
 
== See also ==
 
== See also ==
* [[Project#Developers]]
 
 
* [[WesnothRepository]]
 
* [[WesnothRepository]]
 
+
* [[PatchSubmissionGuidelines]]
 +
* [[SoftwareTesting]]
 +
* [[Project]]
 +
* [[CompatibilityStandards#Deprecation_levels_-_When_to_remove_deprecated_features|DeprecationLevels]]
 
[[Category:Development]]
 
[[Category:Development]]

Revision as of 14:06, 20 October 2019

This page describes guidelines developers with push access to the Wesnoth repository should follow. Most items apply to prospective contributors as well, in addition to the patch submission guidelines.

General

Are you a newcomer to Git or GitHub, or to the Battle for Wesnoth project?
If so, please see WesnothRepository.
  • Set up developer access to the repository, if applicable.
  • Subscribe to the wesnoth-dev mailing list, wherein major announcements are made and project-changing discussions take place.
  • Participate in the #wesnoth-dev IRC channel on irc.freenode.net to coordinate with other developers. Bots report commits and regression testing status, and other developers may ask you questions or provide feedback.

Commits

  • Any given branch should compile without warnings and run after every commit -- we use Travis CI, which will report any build failures on master and maintenance branches (1.12, 1.14, etc.) to the #wesnoth-dev IRC channel.
  • All unit tests should pass after commit -- Travis will also test this. To run the C++ unit tests on your own machine, compile the "test" executable and run it. To run the WML unit tests, run run_wml_tests -u in the checkout root. If a test times out spuriously on Travis, you can restart the build by
    1. Clicking on the link posted to IRC by the Travis bot:
      20180608 07:25:20-!- travis-ci [~travis-ci@ec2-54-204-112-72.compute-1.amazonaws.com] has joined #wesnoth-dev
      20180608 07:25:21< travis-ci> wesnoth/wesnoth#18505 (1.14 - 8722221 : Charles Dang): The build was broken.
      20180608 07:25:21< travis-ci> Build details : https://travis-ci.org/wesnoth/wesnoth/builds/389584322
      20180608 07:25:21-!- travis-ci [~travis-ci@ec2-54-204-112-72.compute-1.amazonaws.com] has left #wesnoth-dev []
    2. Clicking on the restart button on the Travis website.
  • Do not disable or ignore unit tests without a very good reason!
  • A few small commits are better than a single large commit (which is hard to review), so when possible split it in working parts with info about where you are going
  • Always review your changes, both before committing locally and before pushing upstream (see WesnothRepository#Reviewing your changes).
  • Never use the "force" option when pushing to the upstream Wesnoth repositories (see WesnothRepository#Force-pushing_policy).

Changelog and release notes

We provide a changelog at the root of the Wesnoth distribution and Git checkouts. It contains all changes, except for those that are only relevant to mainline developers and which are not expected to impact players or creators in any way.

It is your responsibility to update the changelog as you see fit for every commit. For large changes spread over long series of commits, you will probably prefer to commit your changelog additions in a separate last commit to avoid or mitigate conflicts when merging upstream.

Important changes that might be expected to inconvenience or confuse players (including those building from source) or content creators, major bug fixes, and noteworthy feature additions for a future release should be mentioned and explained in RELEASE_NOTES as well, so that the release team can include this information in official announcements. See the contents of the file for instructions.

Commit messages

A good commit should not only contain good changes, but also include a helpful description of them for other developers, people tracking regressions, project maintainers, and even yourself in the future. There are many style guides on the Web describing best practices for documenting your Git commits.

For Wesnoth in particular, the general consensus is that contributors should adhere to the following rules:

  • Every commit message should begin with a self-contained summary/subject line no more than 72 characters long. This is the first (and often the only) line someone using browsing tools will see. Like with email subjects, this line should not have a trailing stop.
  • If your message needs more than a leading summary line, separate it from the rest with a blank line.
  • Subsequent lines should also be no more than 72 characters long. Use blank lines to separate paragraphs and list items.
  • When working on a mainline campaign/scenario, start the commit message with its acronym. For example HttT: when working on HttT in general and HttT S03: if the commit only applies to the third scenario of HttT (see below).
  • Likewise, if you work on the code of a specific component, you may want to use it as a prefix. That is e.g., gui2: or wesnothd: .
  • If you are performing branch merges, Git may include a list of conflicted paths in the commit message template. Edit this out; it ceases to be interesting after the conflicts have been resolved.
  • If you need to mention commit SHA1 hashes (e.g. in revert messages), make sure to use hashes from the upstream repository that are unlikely to change due to a future local or remote merge or rebase operation.

There are a few additional points to keep in mind for the wording of the contents:

  • Include "bug #1234" somewhere in your commit message if it addresses a bug or feature request from the Wesnoth bug tracker.
  • Use the project's standard abbreviations for campaigns and gameplay concepts, like HttT for Heir to the Throne, ZoC for "Zone of Control", etc. If referring to a particular campaign or scenario in your commit subject, you should use this convention: "HttT S01: Made Galdrad less suicidal"

Dependencies

  • Any changes to Wesnoth's build and run-time dependencies must be discussed on the developers mailing list first.
  • Update the CMake and SCons recipes accordingly.
  • Changes must be mentioned in the RELEASE_NOTES file, so they can be included in the forum announcement for the next release, and in the emails to packagers.

Bugs management

  • Change Status field of fixed bugs to "Fixed" after pushing upstream
  • Change Open/Closed field of fixed bugs to "Closed" after a release including the relevant commits has been published. See ReportingBugs#Bug_protocol for details.
  • Check regularly if there are new bugs relevant to your code and, if any, assign them to you.

See also