Difference between revisions of "SWGANH Development Process"
(→Use Case: Fixing a Bug From Mantis) |
(→TL:DR For Bash Users) |
||
(6 intermediate revisions by the same user not shown) | |||
Line 6: | Line 6: | ||
Many terms found in the article are used in the below documentation such as develop, feature and hotfix branches. Please refer back to the above if further clarification on such terms is needed. Below is the overview image from the above article kept here for easy reference while reading the rest of this document. | Many terms found in the article are used in the below documentation such as develop, feature and hotfix branches. Please refer back to the above if further clarification on such terms is needed. Below is the overview image from the above article kept here for easy reference while reading the rest of this document. | ||
+ | |||
[[Image:GitWorkflow.png]] | [[Image:GitWorkflow.png]] | ||
Line 91: | Line 92: | ||
[[Image:DevProcessBugfix4.png]] | [[Image:DevProcessBugfix4.png]] | ||
− | NOTE: If the change that's being committed is to fix a mistake in the previous commit tick the "Amend Last Commit" and leave the message at the default, this will squash the two commits into one and "hide" the mistake! | + | '''NOTE: If the change that's being committed is to fix a mistake in the previous commit tick the "Amend Last Commit" and leave the message at the default, this will squash the two commits into one and "hide" the mistake!''' |
Once committed the main window will show an updated status that lists the commit hash and log message. | Once committed the main window will show an updated status that lists the commit hash and log message. | ||
Line 102: | Line 103: | ||
By default the current branch should be selected, if not select the current branch and make sure the Remote Destination Repository is set to '''origin''' and that all options in the Transfer Options are unchecked, then hit the Push button. That's it, now the fix is ready for review! Update the related mantis ticket and head to the IRC channel to find a core developer to assist with the Peer Review. | By default the current branch should be selected, if not select the current branch and make sure the Remote Destination Repository is set to '''origin''' and that all options in the Transfer Options are unchecked, then hit the Push button. That's it, now the fix is ready for review! Update the related mantis ticket and head to the IRC channel to find a core developer to assist with the Peer Review. | ||
+ | |||
+ | === TL:DR for Bash Users === | ||
+ | |||
+ | If using the command line the below is a simplified version of the above workflow: | ||
+ | |||
+ | 1) Create a new branch of of develop for the change: | ||
+ | <code> | ||
+ | git checkout -b issue0000001 develop | ||
+ | </code> | ||
+ | |||
+ | 2) List modifications: | ||
+ | <code> | ||
+ | git status | ||
+ | </code> | ||
+ | |||
+ | 3) Add changes to staging area | ||
+ | <code> | ||
+ | git add . | ||
+ | [or] | ||
+ | git add path/to/filename | ||
+ | </code> | ||
+ | |||
+ | 4) Commit code | ||
+ | <code> | ||
+ | git commit -m "Log message" | ||
+ | [or to commit all changed files without explicitly adding them run] | ||
+ | git commit -a -m "Log message" | ||
+ | </code> | ||
+ | |||
+ | 5) Finally push the changes on up | ||
+ | <code> | ||
+ | git push origin issue0000001 | ||
+ | </code> | ||
== Use Case: Peer Reviewing a Bug Fix == | == Use Case: Peer Reviewing a Bug Fix == | ||
Line 116: | Line 150: | ||
In the above diff we note a slight error in the last commit that the original developer overlooked, the log statement has used a %d placeholder used for printing out numbers while a string was passed as an argument. This highlights the point of performing peer reviews, to help in catching mistakes before they ever make it to the main testing branch. Now the reviewer can point out this issue to the original developer who can then implement the necessary fix. Once that's done the issue can be considered to have passed review and the mantis ticket can be updated to state that the fix is ready to be merged into the main testing branch. | In the above diff we note a slight error in the last commit that the original developer overlooked, the log statement has used a %d placeholder used for printing out numbers while a string was passed as an argument. This highlights the point of performing peer reviews, to help in catching mistakes before they ever make it to the main testing branch. Now the reviewer can point out this issue to the original developer who can then implement the necessary fix. Once that's done the issue can be considered to have passed review and the mantis ticket can be updated to state that the fix is ready to be merged into the main testing branch. | ||
+ | |||
+ | === TL:DR for Bash Users === | ||
+ | |||
+ | To do the above in the Bash prompt run the following: | ||
+ | |||
+ | 1) Do a pull to grab the latest code | ||
+ | <code> | ||
+ | git pull | ||
+ | </code> | ||
+ | |||
+ | 2) Checkout the bug branch | ||
+ | <code> | ||
+ | git checkout issue0000001 | ||
+ | </code> | ||
+ | |||
+ | 3) Display the diff of the last X changes relevant to the fix | ||
+ | <code> | ||
+ | git diff HEAD~X | ||
+ | </code> |
Latest revision as of 11:17, 6 April 2010
Contents
Overview of the Git Developer Workflows
Part way into the preparation of this documentation the writer stumbled upon an article just written that precisely describes the workflows that SWG:ANH are embracing. Please read the below article before returning to this documentation.
Many terms found in the article are used in the below documentation such as develop, feature and hotfix branches. Please refer back to the above if further clarification on such terms is needed. Below is the overview image from the above article kept here for easy reference while reading the rest of this document.
Different Branches of the Development and Release Cycle
With a project as large as SWG:ANH, both in terms of developers and in lines of code, it's important to keep a tight reign on the quality of code that makes it to end users. With that in mind the decision was made to adopt a tiered approach to the development/release cycle. This ensures that the master branch is always stable and also gives developers freedom to explore new concepts and ideas without worrying about interrupting the work of others. Finally, it promotes better quality code due to the increased awareness of what other developers are doing and the pooling of the combined knowledge of the development team.
Milestones and Releases
In order for a development project to move forward it has to have something to move forward to, some goal that is the driving force for the developer efforts. The overall goal here at SWG:ANH is to reproduce a functional Star Wars Galaxies PreCU server, this is referred to as the Project Goal. This is quite a large goal though, certainly not one anybody could sit down on a sunday afternoon and hope to complete. Without some way to measure progress developers quickly become disinterested causing the project to stagnate, this is not a good thing.
Dealing with the scope of the Project Goal involves breaking it down into a series of smaller goals, called Milestones. These Milestones serve as a stepping stones from where the project is currently to achieving the Project Goal. They consist of planned feature implementations and fixing known bugs. Once the goals for a particular milestone have been met a Release is made. Each release is tagged with a release number in the form of:
MAJOR RELEASE.MINOR RELEASE.PATCH RELEASE.BUILD NUMBER
What does this number mean though?
MAJOR RELEASE
This part of the build number only increases when the Project Goal is met. Our first Major Release will be the 1.0.0 release.
MINOR RELEASE
Every time a milestone is met the minor release number is increased. At the time of this writing the next minor release will be the 0.2.0 release.
PATCH RELEASE
In between milestones it's likely that there will be bugs found with the previous milestone. Each time a bug is identified, fixed and accepted into a release a Patch Release is created and this number incremented.
BUILD NUMBER
This number is tracked by our internal build server and is incremented each time the code is checked out and built. This number is used internally by QA to keep track of what features and fixes they should be testing at any given time, much like what SVN revision numbers were previously used for.
Tiers of Development
Feature Branches
All development for the project takes place in feature branches, whether it's a new feature or a bug fix. With Git, creating a branch is a simple and lightweight task and switching between various branches is even easier. For an example of the normal developer workflow see the "Use Case: Fixing a bug from manitis" section later in this guide.
Once work has been completed on a feature or a bug has been fixed the changes must be reviewed by at least one other developer on the team. This helps catch mistakes or issues that might not have been apparent to the original developer or perhaps a fresh set of eyes can point out a better path to take. Performing peer reviews also helps everyone to think about problems in new ways and to explore new ideas or solutions they might not have come up with on their own.
After passing peer review the changes are merged into the develop branch where they can be tested by QA.
Release Branches
Every release is the culmination of all the goals that make up a particular milestone. When QA has signed off on all the added features and bug fixes planned for a milestone a release number is picked and a new release branch is created off of the develop branch, this marks the beginning of the release cycle. Each release cycle begins with a four week beta phase, a period of testing of the planned features and fixes of a milestone. At the end of the beta phase (or as soon as all bugs from testing are resolved) the release enters the RC phase. Provided no critical bugs are found at this point for a 2 week period the code is determined ready for release. If any critical bugs are found a new RC is marked and the 2 week incubation period restarts.
Stable (master) Branch
On git the default branch is called the "master" branch. At SWG:ANH this is concidered the Stable branch and it is always the same as the current point-release of the project (@note at the time of this writing we haven't had our first release yet).
After a release branch has passed through it's RC phase of testing it is the Release Manager's duty to merge the release branch into the stable branch and publish all release materials (for example, creating the latest source tarball or windows installer and uploading them to the release site). Once this is done a new release branch is created for development on the next milestone.
This means that only code that has been peer reviewed and signed off by QA ever makes it into the stable master branch, which is important considering this is the branch that will be used by most end users to keep their server's up-to-date with the latest releases.
Use Case: Fixing a Bug From Mantis
Now that a bit of the basics have been covered lets look at a real world example of how this developer workflow works. To do that we'll take a look at one of the more common tasks around here, fixing a bug listed in mantis.
The first thing we'll want to do when beginning to tackle any new problem is to create a new branch. With Git GUI this is easily done by going to Branch -> Create and setting it up as shown in the image below.
Note: All features and bugs must be registered with mantis, the ID number that mantis generates for this is the issue id. This id will then be used in naming your branch in the following manner: issueMANTIS_ID (eg., issue0000001)
Second Note: All feature branches MUST be branched off the 'develop' branch. If this branch isn't showing in your local branches perform the above steps being sure to select the 'Match Tracking Branch Name' option and the 'origin/develop' branch.
This puts the current working directory in the new branch ready for the necessary changes to address the bug at hand. The image below shows the state of Git GUI after making the branch, note that just under the menu the current branch is listed and the status bar at the bottom displays the output of the most recent action.
In this example an issue is fixed with the PingServer project where an error was being logged directly to standard output rather than through the log library. It is a simple fix enough and after making the changes it's time to bring up Git GUI again to save them and share them with the rest of the team. If the Git GUI window hasn't been closed the Rescan button will display the latest state of the git environment as shown below.
The above image shows the file that was modified in the Unstaged Changes section, an unstaged change means that git is aware that a change (or changes) has been made within the repository. The right hand side shows the "diff" of the changes to the selected file, for more information on reading a diff see the Use Case for performing a Peer Review.
In order for a change to be committed it needs to be added to the "staging area", a collection of changes to be included in a commit. This concept may be a bit strange at first, especially for anyone coming over from other version control systems such as SVN but fear not, in time the staging area becomes a handy tool for managing which files do and do not make it into a commit.
To stage a file use the Commit -> Stage to Commit command or it's shortcut Ctrl-T, however, the most common action is to stage all changed files, this can be done using the Stage Changed button on the main Git GUI screen. Once the changes have been staged an appropriate comment should be set and the Commit button hit.
NOTE: If the change that's being committed is to fix a mistake in the previous commit tick the "Amend Last Commit" and leave the message at the default, this will squash the two commits into one and "hide" the mistake!
Once committed the main window will show an updated status that lists the commit hash and log message.
Now that the fix is complete it's ready to be reviewed by another member of the team before being included in the latest development line. To do that hit the Push button on the main screen to bring up the Push dialog and fill it out as shown below.
By default the current branch should be selected, if not select the current branch and make sure the Remote Destination Repository is set to origin and that all options in the Transfer Options are unchecked, then hit the Push button. That's it, now the fix is ready for review! Update the related mantis ticket and head to the IRC channel to find a core developer to assist with the Peer Review.
TL:DR for Bash Users
If using the command line the below is a simplified version of the above workflow:
1) Create a new branch of of develop for the change:
git checkout -b issue0000001 develop
2) List modifications:
git status
3) Add changes to staging area
git add .
[or]
git add path/to/filename
4) Commit code
git commit -m "Log message"
[or to commit all changed files without explicitly adding them run]
git commit -a -m "Log message"
5) Finally push the changes on up
git push origin issue0000001
Use Case: Peer Reviewing a Bug Fix
Core developers are responsible for spending a portion of their time reviewing others code and making themselves available for peer review, this helps to improve the overall quality of code going into the project by forcing developers to think critically about their own work as well as that of others. In this example a bug that was fixed in the previous use case has been updated on the mantis tracker that it's ready for review. To begin reviewing a branch of code perform a pull to make sure you have the latest changes by going to Remote -> Fetch From -> origin. Then create a local tracking branch to review the change by going to Branch -> Create..., the image below shows the options necessary to checkout the fix for issue 0000001.
Once on the correct branch it's time to review the changes. Git GUI makes this a snap with the Repository -> Visualize issue0000001's History command which brings up the window shown below.
The window above provides all the tools necessary to view every change made in the entire history of this line of development! In this example though, only the most recent change is being reviewed and it is selected by default. The bottom left of this screen shows the diff of the changes, a visual depiction of all the changes made in a commit. Each change listed in a diff starts with a tag wrapped in @@ that lists the lines affected by a particular change in the content of a file. Beside each of the lines is either a space, a minus sign or a plus sign. A space beside a line indicates no change while a minus sign indicates a line that has been deleted or modified. Note that lines with a minus sign show the state of the line before a change was made. Plus signs indicate added lines or the new content of changed lines.
In the above diff we note a slight error in the last commit that the original developer overlooked, the log statement has used a %d placeholder used for printing out numbers while a string was passed as an argument. This highlights the point of performing peer reviews, to help in catching mistakes before they ever make it to the main testing branch. Now the reviewer can point out this issue to the original developer who can then implement the necessary fix. Once that's done the issue can be considered to have passed review and the mantis ticket can be updated to state that the fix is ready to be merged into the main testing branch.
TL:DR for Bash Users
To do the above in the Bash prompt run the following:
1) Do a pull to grab the latest code
git pull
2) Checkout the bug branch
git checkout issue0000001
3) Display the diff of the last X changes relevant to the fix
git diff HEAD~X