1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
|
# Code contribution guidelines
To make sure code changes remain easy to organize, find and troubleshoot, please use the following guidelines when contributing code to the Pale Moon project (and UXP). These guidelines may, for some of you, sound tedious or more work than you'd prefer to put in, but keep in mind that a single paragraph of explanation with a code commit can save hours of work later on, and code comments can save your hide when trying to debug or improve code.
## General guideline for issues
Issues are the main drivers behind getting code changes accepted into our source tree. Issues should be opened, and properly labeled, for any of the following:
- Code bugs (broken functionality)
- Regressions (functionality that worked before but has either stopped working, is working in a different way than intended, or e.g. has become poorly performing)
- Enhancements (new code/features)
- Improvements (better/improved way of existing functionality)
- General development direction goals (META issues)
Labels should mostly be assigned to issues, not pull requests (PRs).
Issues opened should, where possible, only address one thing at a time. Never roll multiple bugs or enhancements that aren't directly required to be done at the same time into a single issue.
Issues should include:
### For bugs
- Description of the bug
- Affected versions/features
- Steps to reproduce the bug
- Expected result when performing the steps
- Actual result when performing the steps
### For regressions
- Description of the regression
- Affected versions/features
- What the previous behavior was
- What the current (incorrect) behavior is
### For enhancements and improvements
- Description of the enhancement
- Background: why is this needed or desired?
- Impact of the enhancement/improvement
### META issues
- A general description, including **what** and **why** should be tackled
- Links from the individual components of the META goal (by referencing from the actual individual issues)
For META issues it is important that discussion about specific implementation parts are in the individual component issues and not in the META issue, so it can keep a quick overview of linked components that need to be tackled.
## General guideline for submitting pull requests
Most of our code changes will likely be submitted by collaborators from our community. This will be ported from other projects (e.g. Mozilla-based code) or new implementations, or a mix. The standard way of getting your code into Pale Moon or UXP is by way of creating a pull request against the master branch (trunk).
A good pull request will:
- **Have an issue associated with it!** Even though GitHub offers very similar tools for PRs as it does for issues, any reasoning behind code commits in a PR have to be explained in an accompanying issue.
- Describe in detail **what** the code change attempts to achieve, and **how** it achieves this. If you have references to external sources, e.g. if your code is based on specific code from Mozilla, then including them is of course excellent, but it's important to provide a clear description alongside it.
- Not just be a list of links or references. See the previous point. References are OK but do not provide a reason by themselves for a code change. Especially with porting code across from other projects, it is important to keep PRs and explanations in our tree and not dependent on third-party sources.
- Verify that the code is exactly catered to our tree and in line with our feature set(s) and direction. Just 'porting something across because it is in other browsers" is not a reason or justification by itself. If needed, discuss the **why** for a code change in the attached issue before putting work into a PR that may otherwise be rejected.
- Only address one bug, issue or enhancement. It is extremely important to keep individual code commits separate so that in case of back-outs, this can be done in a controlled and isolated manner.
- If there are dependencies on other code being checked in, it is important that the PR lists exactly which other PRs need to be checked in first before the one in question can be merged. If at all possible, keep PR interdependency to a minimum.
The odd small minor one-liner pull request may not need an issue dedicated to it, but when it comes to modifying and refactoring chunks of many files, then yes it would be not only nice, but required, to have a chance to have discussion about it before it is accepted into the tree.
PRs that encompass major code changes or touch many files or functions should preferably get a review. Reviewing can be volunteered or requested.
## Code Style
Because our software encompasses many components of which a good portion is third-party, code style can vary significantly throughout our code base. It's therefore important to keep code as consistent as possible.
Important when editing/adding code:
1. Stick to the style of surrounding code. If you're editing code in a module that deviates from our generally-used code style, don't mix in generally-used code style with the deviating code style. E.g. if a source file uses tabs everywhere, and you edit this code, also use tabs. If you feel code style should be rewritten to improve legibility, preferably create a separate issue to change it and do not roll it in with functional code changes!
2. Use the following guideline where there's no clear deviating style in use:
- Put opening function/scope braces at the end of the line.
- Align closing braces with the indentation of the first character of the function/scope
- Use a line cap of 80 characters. Put continuations on the next line if it overflows.
- Use spaces for indentation, and indent scopes 2 spaces.
Example:
```C++
nsSVGFilterFrame*
nsSVGFilterFrame::GetReferencedFilterIfNotInUse() {
nsSVGFilterFrame* referenced = GetReferencedFilter();
if (!referenced)
return nullptr;
if (referenced->mLoopFlag) {
// XXXjwatt: we should really send an error to the JavaScript Console here:
NS_WARNING("Filter reference loop detected while inheriting attribute!");
return nullptr;
}
return referenced;
}
```
3. Comment your code where prudent! Documenting what you're doing, especially in a complex piece of code, is very important to keep it easy to debug.
4. Try not to write "as compact as possible" source code. Many languages have facilities to write very compact source code by gluing a lot of statements together on single lines, but you should avoid this to keep it readable. Other people than yourself will be looking at and trying to understand your code, and it's important to keep proper paragraphing, whitespace and above all logical names for variables and functions.
|
