Policies/Commit Policy: Difference between revisions
(git GUI tools) |
m (Fix incorrect spacing after headers) |
||
Line 1: | Line 1: | ||
== Guidelines == | == Guidelines == | ||
=== Think Twice before Committing === | === Think Twice before Committing === | ||
Committing something to KDE's codebase has serious consequences. All other developers will get your changes once they are in the git repository, and if they break something, they will break it for everybody. All commits will be publicly available in the git repository forever. | Committing something to KDE's codebase has serious consequences. All other developers will get your changes once they are in the git repository, and if they break something, they will break it for everybody. All commits will be publicly available in the git repository forever. | ||
Line 7: | Line 9: | ||
=== Never commit code that doesn't compile === | === Never commit code that doesn't compile === | ||
Compile the code and correct all errors before committing. Make sure that newly added files are committed. If they are missing your local compile will work fine but everybody else won't be able to compile. | Compile the code and correct all errors before committing. Make sure that newly added files are committed. If they are missing your local compile will work fine but everybody else won't be able to compile. | ||
Line 14: | Line 17: | ||
=== Test your changes before committing === | === Test your changes before committing === | ||
Start the application affected by your change and make sure that the changed code behaves as desired. | Start the application affected by your change and make sure that the changed code behaves as desired. | ||
Line 19: | Line 23: | ||
=== Double check what you commit === | === Double check what you commit === | ||
Do a <tt>git pull --rebase</tt> to keep your checkout up-to-date. Invoke <tt>git diff</tt> before committing. Take messages from git about conflicts, unknown files, etc. seriously. ''git diff'' will tell you exactly what you will be committing. Check if that's really what you intended to commit. | Do a <tt>git pull --rebase</tt> to keep your checkout up-to-date. Invoke <tt>git diff</tt> before committing. Take messages from git about conflicts, unknown files, etc. seriously. ''git diff'' will tell you exactly what you will be committing. Check if that's really what you intended to commit. | ||
Line 24: | Line 29: | ||
=== Always add descriptive log messages === | === Always add descriptive log messages === | ||
Log messages should be understandable to someone who sees only the log. They shouldn't depend on information outside the context of the commit. Try to put the log messages only to those files which are really affected by the change described in the log message. | Log messages should be understandable to someone who sees only the log. They shouldn't depend on information outside the context of the commit. Try to put the log messages only to those files which are really affected by the change described in the log message. | ||
Line 29: | Line 35: | ||
=== Honor KDE coding policies === | === Honor KDE coding policies === | ||
This includes security (shell quoting, buffer overflows, format string vulnerabilities), binary compatibility (d pointers), i18n, usability, user interface style guide, (API) documentation, documentation and definition of memory management and ownership policies, [[../Frameworks Coding Style/|method naming, conditions for inclusion in KDE Frameworks]], portability issues and license notices. | This includes security (shell quoting, buffer overflows, format string vulnerabilities), binary compatibility (d pointers), i18n, usability, user interface style guide, (API) documentation, documentation and definition of memory management and ownership policies, [[../Frameworks Coding Style/|method naming, conditions for inclusion in KDE Frameworks]], portability issues and license notices. | ||
Line 38: | Line 45: | ||
=== Respect other developer's code === | === Respect other developer's code === | ||
Respect the policies of application and library maintainers, and consult with them before making large changes. | Respect the policies of application and library maintainers, and consult with them before making large changes. | ||
Line 43: | Line 51: | ||
=== Announce changes in advance === | === Announce changes in advance === | ||
When you plan to make changes which affect a lot of different code in KDE's codebase, announce them on the mailing list in advance. For instance, changes in libraries might break other code even if they look trivial, e.g., because an application must also compile with older versions of the library for some reasons. By announcing the changes in advance, developers are prepared, and can express concerns before something gets broken. | When you plan to make changes which affect a lot of different code in KDE's codebase, announce them on the mailing list in advance. For instance, changes in libraries might break other code even if they look trivial, e.g., because an application must also compile with older versions of the library for some reasons. By announcing the changes in advance, developers are prepared, and can express concerns before something gets broken. | ||
=== Code review by other developers === | === Code review by other developers === | ||
Don't commit changes to the public API of libraries without prior review by other developers. There are certain special requirements for the public APIs of the KDE libraries, e.g., source and binary compatibility issues. Changes to the public APIs affect many other KDE developers including third party developers, so requiring a review for these changes is intended to avoid problems for the users of the APIs and to improve the quality of the APIs. | Don't commit changes to the public API of libraries without prior review by other developers. There are certain special requirements for the public APIs of the KDE libraries, e.g., source and binary compatibility issues. Changes to the public APIs affect many other KDE developers including third party developers, so requiring a review for these changes is intended to avoid problems for the users of the APIs and to improve the quality of the APIs. | ||
=== Take responsibility for your commits === | === Take responsibility for your commits === | ||
If your commit breaks something or has side effects on other code, take the responsibility to fix or help fix the problems. | If your commit breaks something or has side effects on other code, take the responsibility to fix or help fix the problems. | ||
=== Don't commit code you don't understand === | === Don't commit code you don't understand === | ||
Avoid things like "I don't know why it crashes, but when I do this, it does not crash anymore." or "I'm not completely sure if that's right, but at least it works for me.". | Avoid things like "I don't know why it crashes, but when I do this, it does not crash anymore." or "I'm not completely sure if that's right, but at least it works for me.". | ||
Line 57: | Line 69: | ||
=== Don't commit if other developers disagree === | === Don't commit if other developers disagree === | ||
If there are disagreements over code changes, these should be resolved by discussing them on the mailing lists or in private, not by forcing code on others by simply committing the changes. | If there are disagreements over code changes, these should be resolved by discussing them on the mailing lists or in private, not by forcing code on others by simply committing the changes. | ||
=== Backport bugfixes === | === Backport bugfixes === | ||
If you commit bugfixes, consider porting the fixes to other branches. Use the same comment for both the original fix and the backport, that way it is easy to see which fixes have been backported already. | If you commit bugfixes, consider porting the fixes to other branches. Use the same comment for both the original fix and the backport, that way it is easy to see which fixes have been backported already. | ||
=== Use bug tracking system numbers === | === Use bug tracking system numbers === | ||
If you fix bugs reported on the bug tracking system, add the bug number to the log message. In order to keep the bug tracking system in sync with the git repositories, you should reference the bug report in your commits, and close the fixed bugs in the bug tracking system. | If you fix bugs reported on the bug tracking system, add the bug number to the log message. In order to keep the bug tracking system in sync with the git repositories, you should reference the bug report in your commits, and close the fixed bugs in the bug tracking system. | ||
Line 68: | Line 83: | ||
=== Tags and branches === | === Tags and branches === | ||
There are rules for where to place release tags and branches in the repository. Official KDE branches and release will be created by the release KDE coordinator in the {{path|branches/KDE}} and {{path|tags/KDE}} directories, scripts will ensure that these folders are protected. | There are rules for where to place release tags and branches in the repository. Official KDE branches and release will be created by the release KDE coordinator in the {{path|branches/KDE}} and {{path|tags/KDE}} directories, scripts will ensure that these folders are protected. | ||
Line 75: | Line 91: | ||
=== Don't add generated files to the repository === | === Don't add generated files to the repository === | ||
Files generated at build time shouldn't be checked into the repository because this is redundant information and might cause conflicts. Only real source files should be in the git repositories. An exception to that are files generated by tools that would be an unusual requirement for building KDE. | Files generated at build time shouldn't be checked into the repository because this is redundant information and might cause conflicts. Only real source files should be in the git repositories. An exception to that are files generated by tools that would be an unusual requirement for building KDE. | ||
Line 82: | Line 99: | ||
=== Commit complete changesets === | === Commit complete changesets === | ||
git has the ability to commit more than one file at a time. Therefore, please commit all related changes in multiple files, even if they span over multiple directories at the same time in the same commit. This way, you ensure that git stays in a compilable state before and after the commit, that the commit history (svn log) is more helpful and that the kde-commits mailing list is not flooded with useless mails. | git has the ability to commit more than one file at a time. Therefore, please commit all related changes in multiple files, even if they span over multiple directories at the same time in the same commit. This way, you ensure that git stays in a compilable state before and after the commit, that the commit history (svn log) is more helpful and that the kde-commits mailing list is not flooded with useless mails. | ||
Line 88: | Line 106: | ||
=== Don't mix formatting changes with code changes === | === Don't mix formatting changes with code changes === | ||
Changing formatting like indenting or white spaces blows up the diff, so that it is hard to find code changes if they are mixed with re-indenting commits or similar things when looking at the logs and diffs later. Committing formatting changes separately solves this problem. | Changing formatting like indenting or white spaces blows up the diff, so that it is hard to find code changes if they are mixed with re-indenting commits or similar things when looking at the logs and diffs later. Committing formatting changes separately solves this problem. | ||
=== GUI Changes === | === GUI Changes === | ||
If your commit causes user visible GUI changes, add the <tt>GUI</tt> keyword to the log message. This makes sure that the documentation writers get notified of your changes. | If your commit causes user visible GUI changes, add the <tt>GUI</tt> keyword to the log message. This makes sure that the documentation writers get notified of your changes. | ||
=== Tag outstanding contributions with Noteworthy === | === Tag outstanding contributions with Noteworthy === | ||
Contributions that bring significant value to our end users should be tagged with the "Noteworthy" label (both for Issues and Merge Requests). This notifies the KDE Promo team about changes to promote and include in announcements. If you are not sure whether a MR or issue should be "Noteworthy" or not, tag it with "Noteworthy" (-> be liberal with the label usage). Promo will then consider such edge cases in detail. Not all things tagged might make it into an official announcement, because of careful prioritization. If you think something was left out that should definitely have been included, reach out on #kde-promo. | Contributions that bring significant value to our end users should be tagged with the "Noteworthy" label (both for Issues and Merge Requests). This notifies the KDE Promo team about changes to promote and include in announcements. If you are not sure whether a MR or issue should be "Noteworthy" or not, tag it with "Noteworthy" (-> be liberal with the label usage). Promo will then consider such edge cases in detail. Not all things tagged might make it into an official announcement, because of careful prioritization. If you think something was left out that should definitely have been included, reach out on #kde-promo. | ||
Line 110: | Line 131: | ||
== Special keywords in GIT and SVN log messages == | == Special keywords in GIT and SVN log messages == | ||
When you commit changes to git (or SVN) you will be asked for a description of your commit. There are several special keywords defined that you can use in this description. These keywords are always uppercase. | When you commit changes to git (or SVN) you will be asked for a description of your commit. There are several special keywords defined that you can use in this description. These keywords are always uppercase. | ||
Line 135: | Line 157: | ||
== Contact == | == Contact == | ||
This page is maintained by [mailto:[email protected] Cornelius Schumacher]. | This page is maintained by [mailto:[email protected] Cornelius Schumacher]. | ||
[[Category:Policies]] | [[Category:Policies]] |
Revision as of 15:21, 28 November 2023
Guidelines
Think Twice before Committing
Committing something to KDE's codebase has serious consequences. All other developers will get your changes once they are in the git repository, and if they break something, they will break it for everybody. All commits will be publicly available in the git repository forever.
On the other hand git allows one to revert changes, so it's possible to recover from mistakes. This is relatively easy for commits to single files but it can also be a significant amount of work for bigger changes. The baseline is: Be aware of the consequences of your commits. Take time to think about them before committing.
Never commit code that doesn't compile
Compile the code and correct all errors before committing. Make sure that newly added files are committed. If they are missing your local compile will work fine but everybody else won't be able to compile.
You certainly should make sure that the code compiles with your local setup. You should also consider what consequences your commit will have for compiling with the source directory being different from the build directory. It would also be nice if it would compile with the --enable-final option, but we don't explicitly support that.
Be especially careful if you change the build system, e.g. Makefiles.
Test your changes before committing
Start the application affected by your change and make sure that the changed code behaves as desired.
Run unit and regression tests (if available) with the ctest command.
Double check what you commit
Do a git pull --rebase to keep your checkout up-to-date. Invoke git diff before committing. Take messages from git about conflicts, unknown files, etc. seriously. git diff will tell you exactly what you will be committing. Check if that's really what you intended to commit.
Here is further help on how to use git. In addition, you can use git graphical tools. The git project contains "git gui" and "gitk". You can use other git GUI tools, e.g. git-cola, gitg. See https://git-scm.com/downloads/guis .
Always add descriptive log messages
Log messages should be understandable to someone who sees only the log. They shouldn't depend on information outside the context of the commit. Try to put the log messages only to those files which are really affected by the change described in the log message.
In particular put all important information which can't be seen from the diff in the log message.
Honor KDE coding policies
This includes security (shell quoting, buffer overflows, format string vulnerabilities), binary compatibility (d pointers), i18n, usability, user interface style guide, (API) documentation, documentation and definition of memory management and ownership policies, method naming, conditions for inclusion in KDE Frameworks, portability issues and license notices.
These policies are defined separately. If in doubt ask on the mailing list.
Respect commit policies set by the Release Plans
Among other things, it means that binary-incompatible changes (BIC) to unreleased APIs in trunk should only happen on Mondays.
Respect other developer's code
Respect the policies of application and library maintainers, and consult with them before making large changes.
Source control systems are not a substitute for developer communication.
Announce changes in advance
When you plan to make changes which affect a lot of different code in KDE's codebase, announce them on the mailing list in advance. For instance, changes in libraries might break other code even if they look trivial, e.g., because an application must also compile with older versions of the library for some reasons. By announcing the changes in advance, developers are prepared, and can express concerns before something gets broken.
Code review by other developers
Don't commit changes to the public API of libraries without prior review by other developers. There are certain special requirements for the public APIs of the KDE libraries, e.g., source and binary compatibility issues. Changes to the public APIs affect many other KDE developers including third party developers, so requiring a review for these changes is intended to avoid problems for the users of the APIs and to improve the quality of the APIs.
Take responsibility for your commits
If your commit breaks something or has side effects on other code, take the responsibility to fix or help fix the problems.
Don't commit code you don't understand
Avoid things like "I don't know why it crashes, but when I do this, it does not crash anymore." or "I'm not completely sure if that's right, but at least it works for me.".
If you don't find a solution to a problem, discuss it with other developers.
Don't commit if other developers disagree
If there are disagreements over code changes, these should be resolved by discussing them on the mailing lists or in private, not by forcing code on others by simply committing the changes.
Backport bugfixes
If you commit bugfixes, consider porting the fixes to other branches. Use the same comment for both the original fix and the backport, that way it is easy to see which fixes have been backported already.
Use bug tracking system numbers
If you fix bugs reported on the bug tracking system, add the bug number to the log message. In order to keep the bug tracking system in sync with the git repositories, you should reference the bug report in your commits, and close the fixed bugs in the bug tracking system.
This doesn't mean that you don't need an understandable log message. It should be clear from the log message what has been changed without looking at the bug report.
Tags and branches
There are rules for where to place release tags and branches in the repository. Official KDE branches and release will be created by the release KDE coordinator in the branches/KDE and tags/KDE directories, scripts will ensure that these folders are protected.
Developers should place all branches which are aimed to be released in branches/appname and name them like the release, e.g. branches/appname/1.5. For all release tags, tags/appname is the right place.
All temporary working branches (which should be deleted again after the work has ended) should be located in branches/work with some name describing both which part of KDE (or which application) is branched and which work is done there. A good example would be branches/work/khtml-paged for a khtml working branch to include paged media support. Bad idea would be something like branches/work/make-it-cool.
Don't add generated files to the repository
Files generated at build time shouldn't be checked into the repository because this is redundant information and might cause conflicts. Only real source files should be in the git repositories. An exception to that are files generated by tools that would be an unusual requirement for building KDE.
Standard tools include gcc, g++, cmake, /bin/sh, perl, moc, uic and the tools part of KDE itself. Tools needed for KDE applications written in non-C++ are allowed as well (e.g. python, ruby).
Tools which shouldn't be a requirement for building KDE include lex/flex, yacc/bison, python, ruby etc.
Commit complete changesets
git has the ability to commit more than one file at a time. Therefore, please commit all related changes in multiple files, even if they span over multiple directories at the same time in the same commit. This way, you ensure that git stays in a compilable state before and after the commit, that the commit history (svn log) is more helpful and that the kde-commits mailing list is not flooded with useless mails.
OTOH, commits should be preferably "atomic" - not splittable. That means that every bugfix, feature, refactoring or reformatting should go into an own commit. This, too, improves the readability of the history. Additionally, it makes porting changes between branches (cherry-picking) and finding faulty commits (by bisecting) simpler. While most KDE developers do not pay much attention to that rule, some do so very much. Consequently, you should be very careful about it when making commits in areas which you do not maintain.
Don't mix formatting changes with code changes
Changing formatting like indenting or white spaces blows up the diff, so that it is hard to find code changes if they are mixed with re-indenting commits or similar things when looking at the logs and diffs later. Committing formatting changes separately solves this problem.
GUI Changes
If your commit causes user visible GUI changes, add the GUI keyword to the log message. This makes sure that the documentation writers get notified of your changes.
Tag outstanding contributions with Noteworthy
Contributions that bring significant value to our end users should be tagged with the "Noteworthy" label (both for Issues and Merge Requests). This notifies the KDE Promo team about changes to promote and include in announcements. If you are not sure whether a MR or issue should be "Noteworthy" or not, tag it with "Noteworthy" (-> be liberal with the label usage). Promo will then consider such edge cases in detail. Not all things tagged might make it into an official announcement, because of careful prioritization. If you think something was left out that should definitely have been included, reach out on #kde-promo.
Examples of noteworthy changes:
- User facing feature additions (e.g. New useful effect added to Kdenlive)
- Big changes in UI (e.g. a KCM is rewritten in QML and now looks distinctively different)
- Long-standing, annoying bugs (e.g. Rework of the previously bug-ridden MTP implementation in KIO)
- Large technology shifts (e.g. Port to Qt 6)
- Significant performance improvements (best paired with concrete numbers, but not necessary)
Examples of changes not considered noteworthy:
- Small UX annoyances and fixes. Whilst those add up to something very important, the individual changes (e.g. "more consistent padding in dialogs") are not interesting to users.
- Shifts in technology that do not affect the behavior of the product (e.g. porting from library X version Y to library X version Y+1)
- Minor changes to tools and backends used in the development process
Special keywords in GIT and SVN log messages
When you commit changes to git (or SVN) you will be asked for a description of your commit. There are several special keywords defined that you can use in this description. These keywords are always uppercase.
For general information on writing good commit messages check out: https://community.kde.org/Infrastructure/GitLab#Write_a_good_commit_message
The following keywords may appear in your commit message - each keyword needs its own line and be followed by a colon (and optionally spaces) separating it from an argument value (such as bug number):
- CHANGELOG: ['optional one line description']
The optional one line description will be used to fill automatically the release changelog. If it is not there the first line of the commit will be used. If it is fixing a bug, there's no need to mention the bug number if you are already using the BUG: keyword - FEATURE: [<bugnumber>]
Marks the feature as implemented by CC'ing the commit message to <bugnumber>[email protected]. This keyword will also be used to automatically extract entries for the release changelog, so it makes sense to use it for new features even if you don't have a bugnumber for the feature. - BUG: <bugnumber>
Marks the bug as fixed by CC'ing the commit message to <bugnumber>[email protected]. This keyword will also be used to automatically extract entries for the release changelog. To mark multiple bugs, make a BUG: <bugnumber> entry on separate lines for each.- FIXED-IN: <version>
If the BUG keyword is used to close a bug, this keyword will update the bug's release version that the fix appears in. You can only select one version, so prefer the lowest release version that will actually have the fix.
- FIXED-IN: <version>
- CCBUG: <bugnumber>
CC's to the bugreport by sending mail to <bugnumber>@bugs.kde.org - CCMAIL: <email-address>
CC's to the given e-mail address. - Differential Revision: https://phabricator.kde.org/D<number>
Links the commit to the Differential on Phabricator and closes it. It *must* be the last line of the commit message. - GUI:
Indicates a user visible change in the user interface. This is used to make the documentation team aware of such changes.
This keyword can appear anywhere on a line:
- SVN_SILENT
Marks the commit message "silent" by adding "(silent)" to the subject of the mail to allow filtering out trivial commits. Use this tag carefully and only for really uninteresting, uncontroversial commits.
- GIT_SILENT
Same as SVN_SILENT
- SVN_MERGE
Special keyword for people that use the svnmerge script. Marks the commit message as being a merge commit by adding "(merge)" to the subject of the mail. This way receivers can filter out mails that are caused by merging the same patch from one branch to another branch. Reviews of those mails is usually not needed. This keyword filters out the endless property changes used by this script.
- NO_CHANGELOG
To avoid the commit being mentioned in changelogs.
Contact
This page is maintained by Cornelius Schumacher.