Changeset View
Standalone View
HACKING.md
Show First 20 Lines • Show All 85 Lines • ▼ Show 20 Line(s) | 85 | KWin uses [KDE's phabricator instance](https://phabricator.kde.org) for code review. Patches can be uploaded automatically using the tool arcanist. A possible workflow could look like: | |||
---|---|---|---|---|---|
86 | 86 | | |||
87 | git checkout -b my-feature-branch | 87 | git checkout -b my-feature-branch | ||
88 | git add ... | 88 | git add ... | ||
89 | git commit | 89 | git commit | ||
90 | arc diff | 90 | arc diff | ||
91 | 91 | | |||
92 | More complete documentation can be found in [KDE's wiki](https://community.kde.org/Infrastructure/Phabricator). Please add "#KWin" as reviewers. Please run KWin's automated test suite prior to uploading a patch to ensure that the change does not break existing code. | 92 | More complete documentation can be found in [KDE's wiki](https://community.kde.org/Infrastructure/Phabricator). Please add "#KWin" as reviewers. Please run KWin's automated test suite prior to uploading a patch to ensure that the change does not break existing code. | ||
93 | 93 | | |||
94 | # Coding style | 94 | ## Coding style | ||
95 | KWin code follows the [Frameworks coding style](https://techbase.kde.org/Policies/Frameworks_Coding_Style). | 95 | KWin code follows the [Frameworks coding style](https://techbase.kde.org/Policies/Frameworks_Coding_Style). | ||
96 | | ||||
97 | ## Commit message guideline | ||||
98 | KWin uses a strict guideline (since August 2019) to improve Git history readability and simplify contributing. Most of this commit message guideline is based on [Angular's one](https://github.com/angular/angular/blob/2be061a96ed35a5157546dee9e7220c964ff4a28/CONTRIBUTING.md#commit). | ||||
99 | ### Commit message format | ||||
100 | Each commit message consists of a **header**, a **body** and a **footer**. The header has a special | ||||
101 | format that includes a **type**, a **scope** and a **subject**: | ||||
102 | | ||||
103 | ``` | ||||
104 | <type>(<scope>): <subject> | ||||
105 | <BLANK LINE> | ||||
106 | <body> | ||||
107 | <BLANK LINE> | ||||
108 | <footer> | ||||
109 | ``` | ||||
110 | | ||||
111 | The **header** is mandatory and the **scope** of the header is optional. Any line of the commit message cannot be longer 100 characters. | ||||
112 | | ||||
113 | ### Revert | ||||
114 | If the commit reverts a previous commit, it should begin with `revert: `, followed by the header of the reverted commit. In the body it should say: `This reverts commit <hash>.`, where the hash is the SHA of the commit being reverted. | ||||
115 | | ||||
116 | ### Type | ||||
117 | Must be one of the following: | ||||
118 | | ||||
119 | * **build**: changes that affect the build system or external dependencies (example scopes: ...) | ||||
120 | * **docs**: documentation only changes | ||||
zzag: Perhaps we could omit `ci` for now. | |||||
121 | * **feat**: a new feature | ||||
122 | * **fix**: a bug fix | ||||
123 | * **perf**: a code change that improves performance | ||||
124 | * **refactor**: a code change that neither fixes a bug nor adds a feature | ||||
125 | * **style**: changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc) | ||||
126 | * **test**: adding missing tests or correcting existing tests | ||||
127 | | ||||
128 | ### Scope | ||||
129 | The scope should be the name or location of KWin's internal module or plugin affected (as perceived by the person reading the changelog generated from commit messages. | ||||
130 | | ||||
131 | Locations composited from paths are denoted with hyphens. For example the following scopes are sensible: | ||||
zzag: Perhaps slashes are better than hyphens as we are more used to it. | |||||
Slashes are used to indicate alternatives in normal text. That's why I opted for some other symbol. romangg: Slashes are used to indicate alternatives in normal text. That's why I opted for some other… | |||||
132 | | ||||
133 | * **workspace** | ||||
134 | * **compositor** | ||||
zzag: These two are not modules. | |||||
I haven't looked up the definition of a "module". Is it something specific? Let's just say topic as you recommended in the other comment? Or "area of interest"? romangg: I haven't looked up the definition of a "module". Is it something specific? Let's just say… | |||||
zzag: > Or "area of interest"?
Yep, this sounds good. | |||||
135 | * **effects** | ||||
136 | * **effects-blur** | ||||
137 | * **platforms-drm** | ||||
138 | * **platforms-x11** | ||||
139 | | ||||
140 | If a single commit touches several different parts of KWin equally naming a scope should be omitted. | ||||
141 | | ||||
142 | ### Subject | ||||
143 | The subject contains a succinct description of the change: | ||||
144 | | ||||
145 | * use the imperative, present tense: "change" not "changed" nor "changes" | ||||
146 | * don't capitalize the first letter | ||||
147 | * no dot (.) at the end | ||||
What's the main argument for keeping the first letter in lower case? If the first letter is not capitalized, then <subject> blends with <type>(<scope>):. IMHO, fix: Add more bugs to fix later is much better than fix: add more bugs to fix later. zzag: What's the main argument for keeping the first letter in lower case?
If the first letter is… | |||||
I assume it's because in English language you are not supposed to capitalize after colon. I don't directly prefer one over the other after using the non-capitalized way in English text now for quite some time but preferred the capitalized way in the beginning since I was used to it from German. romangg: I assume it's because in English language you are not supposed to capitalize after colon. I… | |||||
148 | | ||||
149 | ### Body | ||||
150 | Just as in the **subject**, use the imperative, present tense: "change" not "changed" nor "changes". | ||||
151 | The body should include the motivation for the change and contrast this with previous behavior. | ||||
152 | | ||||
153 | ### Footer | ||||
154 | As the first part of the footer additional information should be added according to the KDE Commit Policy on [special keywords](https://community.kde.org/Policies/Commit_Policy#Special_keywords_in_GIT_and_SVN_log_messages). | ||||
yurchor: Typo: As first part -> As the first part | |||||
155 | | ||||
156 | The rest of the footer currently must be in the format as generated by Phabricator's `arc` tool. In the future after KWin has been transitioned to KDE's GitLab instance the following holds instead: | ||||
yurchor: Typo: ark -> arc | |||||
157 | | ||||
158 | The footer should contain a reference to the GitLab issues that this commit **Closes**. | ||||
The "Breaking Changes" was copied over from the Angular guide lines, but maybe we should remove it here. For once because we don't really have defined what breaking changes are in KWin. Is a change to the Effects API/ABI a breaking change? Is a behavioral change for users one? romangg: The "Breaking Changes" was copied over from the Angular guide lines, but maybe we should remove… | |||||
If KWin loads all its modules at start it sounds unchanged to users, otherwise breaking API/ABI will result in crash at time to load an incompatible plugin. anthonyfieroni: If KWin loads all its modules at start it sounds unchanged to users, otherwise breaking API/ABI… | |||||
We can drop this section as we check libkwineffects API version.
Third-party effects need to be recompiled if API version of libkwineffects is bumped. It's a good idea to compile your third-party effect against each major release of KWin. zzag: We can drop this section as we check libkwineffects API version.
> Is a change to the Effects… | |||||
yurchor: Typo: with a space -> with space |
Perhaps we could omit ci for now.