mirror of https://github.com/vitalif/phantomjs
89 lines
4.6 KiB
Markdown
89 lines
4.6 KiB
Markdown
![]() |
# Contribution Guide
|
||
|
|
||
|
This page describes how to contribute changes to PhantomJS.
|
||
|
|
||
|
Please do **not** create a pull request without reading this guide first. Failure to do so may result in the **rejection** of the pull request.
|
||
|
|
||
|
## For The Impatients
|
||
|
|
||
|
**Work on a feature branch**.
|
||
|
If your changes need to be modified due to some reviews, it is less clutter to tweak an isolated feature branch and push it again.
|
||
|
|
||
|
**Create a ticket in the issue tracker**.
|
||
|
This serves as a placeholder for important feedback, review, or any future updates. Using GitHub commenting system is not optimal because it is *not searchable* and therefore impossible to locate again.
|
||
|
|
||
|
In the commit message:
|
||
|
|
||
|
* **Keep the first line < 72 characters**. Write additional paragraphs
|
||
|
if necessary.
|
||
|
* **Put the link to the issue** (see above). This is important for cross-referencing purposes.
|
||
|
|
||
|
## Communicate
|
||
|
|
||
|
*Second opinion is always important.*
|
||
|
|
||
|
If you have a fix for a bug, please attach your patch in the corresponding issue in the [issue tracker](http://issues.phantomjs.org/). If there is no entry for the bug yet, then please create a new one. If you are convenient working with Git, see the Get Ready section below on how to submit your change.
|
||
|
|
||
|
If you have an improvement idea, please send an email to the [mailing list](http://groups.google.com/group/phantomjs) (preferable than contacting the developers directly) so that other people can give their insights and opinions. This is also important to avoid duplicate work.
|
||
|
|
||
|
Once the feature idea is agreed upon and translated into concrete actions and tasks, please use the [issue tracker](http://issues.phantomjs.org/) to create an issue for each individual task. Further technical discussion about the task and the implementation details should be carried out in the issue tracker.
|
||
|
|
||
|
Whenever you want to introduce a new API, please send an email to the mailing list along with the link to the issue. Consider good API name for the object or function, read the [API Design Principle](http://developer.qt.nokia.com/wiki/API_Design_Principles) article. It may require few iterations to agree on the final API and hence it is important to engage all interested parties as early as possible.
|
||
|
|
||
|
## Get Ready
|
||
|
|
||
|
For your proposed change, you need to have:
|
||
|
|
||
|
* **an issue** (in the issue tracker) which describe your bug or feature
|
||
|
* **a feature branch** in your git fork
|
||
|
|
||
|
### Refer the Issue
|
||
|
|
||
|
The commit message needs to link to the issue. This cross-reference is [very important](http://ariya.ofilabs.com/2012/01/small-scale-software-craftsmanship.html) for the following reasons.
|
||
|
|
||
|
First, the commit log is frozen and can not be changed. If it contains a mistake or outdated information, the log can not be amended. However, further updates can be still posted to the linked issue, which can be followed from the commit log itself.
|
||
|
|
||
|
Second, it provides a placeholder for code review and other feedback. Reviewing the change using GitHub commenting system is not encouraged because it is [not searchable](http://ariya.ofilabs.com/2012/08/github-and-lack-of-searchability.html) at all (if there is a future need to refer the previous discussion again, the comment is almost impossible to locate)
|
||
|
|
||
|
An example of a bad commit log:
|
||
|
|
||
|
Fix Mountain Lion
|
||
|
|
||
|
The above log is too short and useless in the long run. A better version (and note the issue link):
|
||
|
|
||
|
Better support for OS X Mountain Lion.
|
||
|
|
||
|
require('system').os.version should give "10.8 (Mountain Lion)".
|
||
|
|
||
|
http://code.google.com/p/phantomjs/issues/detail?id=688
|
||
|
|
||
|
### Use Feature Branch
|
||
|
|
||
|
To isolate your change, please avoid working on the master branch. Instead, work on a *feature branch* (often also known as *topic branch*). You can create a new branch (example here crash-fix) off the master branch by using:
|
||
|
|
||
|
git checkout -b crash-fix master
|
||
|
|
||
|
Refer to your favorite Git tutorial/book for further detailed help.
|
||
|
|
||
|
Some good practices for the feature branch:
|
||
|
|
||
|
* Give it a meaningful name instead of, e.g. `prevent-zero-divide` instead of just `fix`
|
||
|
* Make *granular* and *atomic* commits, e.g. do not mix a typo fix with some major refactoring
|
||
|
* Keep one branch for one specific issue. If you need to work on other unrelated issues, create another branch.
|
||
|
|
||
|
## Review and Merge
|
||
|
|
||
|
When your branch is ready, send the pull request.
|
||
|
|
||
|
While it is not always the case, often it is necessary to improve parts of your code in the branch. This is the actual review process.
|
||
|
|
||
|
Here is a check list for the review:
|
||
|
|
||
|
* It does not break the test suite
|
||
|
* There is no typo
|
||
|
* The coding style follows the existing one
|
||
|
* There is a reasonable amount of comment
|
||
|
* The license header is intact
|
||
|
* All examples are still working
|
||
|
|