Dev:Git Guidelines: Difference between revisions
| [unchecked revision] | [checked revision] |
(Created page with " == Tips for making good commits == === Never commit code that doesn't validate === Validate the code (XSD-files) and examples (XML-files) together. Correct all errors before...") |
(Reviewed with minor changes) |
||
| (5 intermediate revisions by one other user not shown) | |||
| Line 1: | Line 1: | ||
== Tips for making good commits == | == Tips for making good commits == | ||
=== Master/development branches === | |||
Only commit to the master branch directly if the changes your are commiting are complete in themselves and don't change the syntax of {{rml}}. Changes to the syntax of {{rml}} should be isolated in a development branch, such as <code>railML-2.5-dev</code> and merged back to master along with the necessary documentation upon release of a new version. Changes to the [[Dev:codelists|codelists]], the xsd documentation or the html documentation are permitted to be commited on master. | |||
=== Create feature branches when developing complex changes === | |||
Since complex changes usually are done in more than a single commit it is best to isolate them in a feature branch to not contaminate the branches that are of a more general nature with intermediate solutions. Merge back your changes when you are sure everything is in order, validated and discussed with your team members. Merging back should take place with a merge request using the appropriate [https://development.railml.org/railml/version2/-/merge_requests form for {{rml}} 2] or [https://development.railml.org/railml/version3/-/merge_requests for {{rml}} 3] respectively. | |||
=== Naming branches === | |||
The branch name should describe clearly and concisely what the branch contains or what you are aiming to do in it. Apart from the master, all official {{rml}} branches and tags are named so that they start with <code>railML</code> and the version number. For new feature branches the prefix <code>feature-</code> shall be used. | |||
=== Make "atomic" commits === | |||
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 the code in the repository stays in a valid state before and after the commit. | |||
If your changes affect any example files, please consider adapting the examples as well. | |||
If changes to a file address multiple issues, please try to split them by only staging some of the changes in a file for commit and isolate changes in separate commits if they do now belong together. | |||
=== Never commit code that doesn't validate === | === Never commit code that doesn't validate === | ||
Validate the code (XSD-files) and examples (XML-files) together. Correct all errors before committing. Make sure that newly added files are committed. If they are missing, your local validation will work fine, but everybody else won't be able to validate without errors. | Validate the code (XSD-files) and examples (XML-files) together. Correct all errors before committing. Make sure that newly added files are committed. If they are missing, your local validation will work fine, but everybody else won't be able to validate without errors. You can easily check this with the command <code>git status</code>. | ||
=== Double check what you commit === | === Double check what you commit === | ||
Stage your changes carefully and review the changes before commiting. You can for example easily do that using <code>git gui</code>. If you made changes that are related to different issues in the same file please also remember to stage/unstage only certain chunks/lines of your changes that relate to a single issue. | |||
=== Carefully resolve conflicts === | |||
If you cannot push your commits and run into conflicts when reintegrating the remote state into your local git, take your time to carefully resolve conflicts that may arise. Other commiters may have also added valuable content so simply resolving a conflict by rejecting "Theirs" is not a solution. If necessary contact the other comitters to find a solution. | |||
=== Always add descriptive log messages === | === Always add descriptive log messages === | ||
Write commit messages in English. | |||
In particular put all important information which can't be seen from the diff in the log message. | |||
=== Respect other | The first line of commit is a summary of the changes. The rest of the message should give more details on the change as appropriate. Give credit where credit is due. Please do not forget to include the reasoning for the change. This will help others (and yourself) understand the code later. | ||
Commit messages should be understandable to someone who sees only the history. They shouldn't depend on information outside the context of the commit. Try to include only those files and lines which are really affected by the change described in the log message, and make separate commits for other changes. | |||
In particular put all important information which can't be seen from the diff in the log message, most importantly why a certain change was done and was necessary. | |||
=== Reference issue trackers === | |||
When commiting, include a reference to an issue from [https://development.railml.org/railml/version2/-/issues the issue tracker for {{rml}} 2] or [https://development.railml.org/railml/version3/-/issues for {{rml}} 3] in the commit message. For {{rml}} 2, you can do so by simply mentioning the issue number prefixed with a <code>#</code>, e.g. <code>#42</code>. For {{rml}} 3, as the development is performed on the UML model in [https://development.railml.org/railml/railml3-ea the <code>railML3 EA</code> project], while the issues are in [https://development.railml.org/railml/version3 the <code>railML 3</code> project], you need to prefix the issue number with <code>version3#</code>, e.g. <code>version3#42</code>, when you commit to <code>railML3 EA</code>. | |||
=== Respect other developers' code === | |||
Consult other developers before making large changes. Source control systems are not a substitute for developer communication. | Consult other developers before making large changes. Source control systems are not a substitute for developer communication. | ||
=== Announce changes in advance === | === Announce changes in advance === | ||
When you plan to make changes which affect a lot of different code in the repository, announce them in the | When you plan to make changes which affect a lot of different code in the repository, announce them in the relevant forum or working group in advance. By announcing the changes in advance, the other developers are prepared, and can avoid making conflicting changes or wasted overlapping work. Prepare those changes in a branch and have others take a look at your proposed solution so that nothing gets broken and no intermediate states find their way to the more general branches. | ||
=== 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. | ||
=== Backport bugfixes === | |||
If you commit bugfixes, consider porting the fixes to other branches. The easiest way to do that is by using a <code>cherry-pick</code>. You can do this on the command line or with various graphical tools. The git setup comes with <code>gitk --all</code> that easily allows this. Switch to the branch you want to apply the bugfix to, run <code>gitk --all</code>, select the commit from the other branch (where you already fixed the problem), right-click and select "Cherry-pick". Resolve potential conflicts. | |||
=== Don't commit code you don't understand === | === Don't commit code you don't understand === | ||
Avoid things like "I'm not completely sure if that's right, but at least it works for me." | Avoid things like "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. | If you don't find a solution to a problem, discuss it with other developers. | ||
=== 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 in the | If there are disagreements over code changes, these should be resolved by discussing them in the relevant forum or working grop, or in private (e-mail or phone), not by forcing code on others by simply committing the changes to the repository. | ||
=== 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. | ||
== Review process == | == Review process == | ||
Commits are reviewed by the | Commits are reviewed by the {{rml}} coordinators. In order to have {{rml}} coordinators review your changes please issue a merge request via the appropriate [https://development.railml.org/railml/version2/-/merge_requests form for {{rml}} 2] or [https://development.railml.org/railml/version3/-/merge_requests for {{rml}} 3] respectively. In case of commits, non-conformant with these guidelines, railML coordinators have to fix the situation with a commit indicating the correction in the log message. | ||
[[Category:GeneralDescription]] | [[Category:GeneralDescription]] | ||
{{interwiki}} | {{interwiki}} | ||
Latest revision as of 13:18, 26 July 2023
Tips for making good commits
Master/development branches
Only commit to the master branch directly if the changes your are commiting are complete in themselves and don't change the syntax of railML®. Changes to the syntax of railML® should be isolated in a development branch, such as railML-2.5-dev and merged back to master along with the necessary documentation upon release of a new version. Changes to the codelists, the xsd documentation or the html documentation are permitted to be commited on master.
Create feature branches when developing complex changes
Since complex changes usually are done in more than a single commit it is best to isolate them in a feature branch to not contaminate the branches that are of a more general nature with intermediate solutions. Merge back your changes when you are sure everything is in order, validated and discussed with your team members. Merging back should take place with a merge request using the appropriate form for railML® 2 or for railML® 3 respectively.
Naming branches
The branch name should describe clearly and concisely what the branch contains or what you are aiming to do in it. Apart from the master, all official railML® branches and tags are named so that they start with railML and the version number. For new feature branches the prefix feature- shall be used.
Make "atomic" commits
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 the code in the repository stays in a valid state before and after the commit.
If your changes affect any example files, please consider adapting the examples as well.
If changes to a file address multiple issues, please try to split them by only staging some of the changes in a file for commit and isolate changes in separate commits if they do now belong together.
Never commit code that doesn't validate
Validate the code (XSD-files) and examples (XML-files) together. Correct all errors before committing. Make sure that newly added files are committed. If they are missing, your local validation will work fine, but everybody else won't be able to validate without errors. You can easily check this with the command git status.
Double check what you commit
Stage your changes carefully and review the changes before commiting. You can for example easily do that using git gui. If you made changes that are related to different issues in the same file please also remember to stage/unstage only certain chunks/lines of your changes that relate to a single issue.
Carefully resolve conflicts
If you cannot push your commits and run into conflicts when reintegrating the remote state into your local git, take your time to carefully resolve conflicts that may arise. Other commiters may have also added valuable content so simply resolving a conflict by rejecting "Theirs" is not a solution. If necessary contact the other comitters to find a solution.
Always add descriptive log messages
Write commit messages in English.
The first line of commit is a summary of the changes. The rest of the message should give more details on the change as appropriate. Give credit where credit is due. Please do not forget to include the reasoning for the change. This will help others (and yourself) understand the code later.
Commit messages should be understandable to someone who sees only the history. They shouldn't depend on information outside the context of the commit. Try to include only those files and lines which are really affected by the change described in the log message, and make separate commits for other changes.
In particular put all important information which can't be seen from the diff in the log message, most importantly why a certain change was done and was necessary.
Reference issue trackers
When commiting, include a reference to an issue from the issue tracker for railML® 2 or for railML® 3 in the commit message. For railML® 2, you can do so by simply mentioning the issue number prefixed with a #, e.g. #42. For railML® 3, as the development is performed on the UML model in the railML3 EA project, while the issues are in the railML 3 project, you need to prefix the issue number with version3#, e.g. version3#42, when you commit to railML3 EA.
Respect other developers' code
Consult other developers 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 the repository, announce them in the relevant forum or working group in advance. By announcing the changes in advance, the other developers are prepared, and can avoid making conflicting changes or wasted overlapping work. Prepare those changes in a branch and have others take a look at your proposed solution so that nothing gets broken and no intermediate states find their way to the more general branches.
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.
Backport bugfixes
If you commit bugfixes, consider porting the fixes to other branches. The easiest way to do that is by using a cherry-pick. You can do this on the command line or with various graphical tools. The git setup comes with gitk --all that easily allows this. Switch to the branch you want to apply the bugfix to, run gitk --all, select the commit from the other branch (where you already fixed the problem), right-click and select "Cherry-pick". Resolve potential conflicts.
Don't commit code you don't understand
Avoid things like "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 in the relevant forum or working grop, or in private (e-mail or phone), not by forcing code on others by simply committing the changes to the repository.
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.
Review process
Commits are reviewed by the railML® coordinators. In order to have railML® coordinators review your changes please issue a merge request via the appropriate form for railML® 2 or for railML® 3 respectively. In case of commits, non-conformant with these guidelines, railML coordinators have to fix the situation with a commit indicating the correction in the log message.