Git Workflow¶
This page outlines the Git workflow used for the CTA project. By following this workflow, we can ensure a smooth and efficient development process with minimal conflicts and a clean history.
Initial setup¶
Before you start your development. Git should be configured to use your CERN email and username using the following commands:
git config --global user.email "<YOUR CERN EMAIL>"
git config --global user.name "<YOUR CERN USERNAME>"
Optionally (but recommended), configure which text editor git should be using for any text operations:
git config --global core.editor "vim"
For setting up a development environment, see the section on Development Setup.
Trunk-based Development¶
For our version control management practice we use trunk-based development. This means we have a single main
branch where we frequently merge small features, bug fixes and maintenance updates. It is important that changes are kept small and focussed.
The branch main
is protected. Therefore, it is not possible to directly push new commits. The only way to merge the changes is with a Merge Request (MR).
Before a branch is merged into the main
branch, it should be reviewed and approved by one other member of the development team.
The general workflow for development work is as follows:
- An issue is created for the feature/bug/maintenance in question. See Development Tracking for how to create a proper issue.
- Before moving on, it should be discussed with the dev team whether this issue is something to implement.
- The
~"workflow::backlog"
label should be assigned if it has been decided that this issue is something to work on. - Create a branch from the issue. Do this by clicking on the arrow next to
Create merge request
. This way the branch name will include the issue number and title so that it is clear what this branch is for. If you create the branch through the cli (which is perfectly fine), ensure the branch follows the exact same naming conventions. Optionally, create a draft merge request from the created branch intomain
. - The
~"workflow::in progress"
label should be assigned when work has started on the issue. - The
~"workflow::testing"
label should be assigned when the main work has finished, but the developer is still testing functionality (either in CI or manually). - The
~"workflow::in review"
label should be assigned when the work has finished and it is awaiting review. - If a review requires changes, move it back to one of the previous states as required.
- If at any point the issue cannot progress due to another issue it should have the
~"workflow::blocked"
label.
Info
Developers should regularly review their open tickets to ensure that the workflow::
tag accurately reflects the state of the issue, and that the issue is closed after it is finished.
Branches¶
Branch names should only contain alpha-numeric (lowercase) characters and hyphens (-
). If a branch name should include a version number, using dots (.
) is allowed as well.
The structure of the branch name should be:
<issue-number>-<issue-title>
This will be done automatically if you create the branch from the issue itself using the GitLab, which is the recommended way. In certain select scenarios, it might be necessary to create a branch without associating it to an issue (there should be a good reason for this). In this case, choose a clear and descriptive name. If you have a test-branch for a given issue, re-use the branch name above and add a descriptive suffix. Do not create random test branches without a proper name! Ensure any test branches are cleaned up when they are no longer needed.
Finally, ensure you frequently rebase your branches to prevent having to do a potentially complicated rebase at the very end.
Merge Requests¶
No pushing is allowed directly to main
, so any update to the CTA codebase must go through a merge request.
The developer should:
- Review the MR title.
- The title should be clear, concise and suitable as a changelog entry. By default, the title of the MR will become the commit summary in the squashed commit.
- Important: As a general rule:
MR title = squashed commit message = changelog entry
. As such, carefully read through theWriting good changelog entries
section to write a good MR title - Don't forget to remove the
Resolve:
prefix!
- Fill the description:
- Add a summary/list of the changes that were made. Developers looking at the MR should get an idea of what the MR updated just by looking at the description.
- Clarify if the MR requires any documentation updates using the checkbox. If so, create an issue in the
https://gitlab.cern.ch/cta/eoscta-docs
repo. Make sure that the documentation is updated before or shortly after the MR is merged. - Provide links to the corresponding issue if not already present.
- Ensure the changes are finalised and that the CI passes.
- Assign a reviewer:
- Usually, the reviewer is simply the next name on a "round robin" list of CTA developers, so code reviews are spread evenly around the team.
- The developer can choose a specific reviewer if they want, for example because they want the opinion of someone with a particular expertise.
- Wait for a review and resolve any issues raised by the reviewer.
If the MR has been approved and the CI pipeline passes the MR is ready to be merged. After this, the developer can merge the branch to main
. Commits should be squashed when merging, so that only a single commit is merged.
- When merging the changes back into
main
, the branch must be rebased on top ofmain
(git rebase origin/main
). The changes should then be retested, and any conflicts resolved (see git rebase documentation). - If needed, push the rebased feature branch with
git push --force-with-lease
. Be aware that pushing with--force
is dangerous and potentially destructive! Make sure to use --force-with-lease to (slightly) reduce these dangers. - After the MR has been merged or closed, the issue should be closed as well if not done automatically.
The reviewer should
- Check that the MR title is clear, concise and suitable as a changelog entry.
- Even if the MR contains only developer-facing changes, the MR title (and consequently the squashed commit message) should read as a changelog entry. This ensures that viewing the commit history gives developers a good idea of all the changes that happened (not just the user-facing changes).
- Confirm that the MR description summarises the changes correctly.
- Confirm whether documentation changes are needed.
- Look at the code changes and make comments.
- Approve the MR once all issues raised by the reviewer have been addressed
Changelog¶
The changelog is used to convey relevant changes in CTA for a given release to the user. Please carefully read the sections below.
What warrants a changelog entry¶
- Any user-facing change should have a changelog entry. Examples:
- Feature additions/changes
- Most bug fixes
- Changes in the config file structure or commandline interface of the various CTA packages
- Name changes to a CTA package
- Any developer-facing change should not have a changelog entry. Examples:
- Changes to the CI
- Update to README files
- Updates to the tests
- Refactorings
- If a feature is introduced and then fixed or changed within the same release, the fix/change should not have a changelog entry.
Remember, the changelog is for users to understand what changed in the software between releases. For a full overview of what changed, developers can always look at the commit history.
Writing good changelog entries¶
Changelog entries should:
- start with a verb in the imperative mood. If in doubt, just think: "this commit will
<commit-title>
" Some examples:- Good: [rmcd] Fix smc request handling always hitting 5s timeout
- Good: [scheduler] Update scheduler to handle multiple frontend connections
- Bad: Fixed issue where logs were not written correctly
-> past tense, not imperative
- Bad: Retry logic for repack implemented
-> noun phrase, not imperative
- Bad: Fixes retry logic
-> third person, not imperative
- Bad: Adding new support for something
-> gerund, not imperative
- be concise and descriptive. Examples:
- Good: [taped] Add support for log rotation on CTA tape servers
- Good: [Misc] Bump EOS version to 5.8.2
- Bad: Logging improvements
-> vague
- Bad: Miscellaneous bug fixes and improvements
-> meaningless
- Bad: Fix queueing issues
-> not descriptive about what issue is being solved
- focus on the end-result instead of the implementation
- Good: [frontend] Fill xrd::cta::response field in case of grpc error
- Good: [taped] Fix taped core dumping due to logging concurrent modifications
- Bad: Refactored the repack manager class to add a new retry field
-> too many implementation details
- Bad: Updated variable names for consistency
-> unless it affects behavior, not user-relevant
- Bad: Used a try-catch block in logwriter.cpp
-> talks about how, not what or why
- not have unnecessary capitalization
- not contain spelling errors
- not start with
Resolve "
Note that some of the above examples could be a bit more descriptive. However, try to keep the entries/commit summaries under 72 characters in length.
It is typically useful to know the scope of the commit. As such, adding a prefix is recommended if suitable. Currently, we have the following (non-exhaustive) list of prefixes:
[CI] My commit message
[Misc] My commit message
[Tools] My commit message
[catalogue] My commit message
[frontend] My commit message
[scheduler] My commit message
[taped] My commit message
[rmcd] My commit message
For changes that are mechanical, stylistic, or span multiple components (e.g., formatting, logging adjustments, or comment changes), use the [Misc]
prefix. If a change is logically scoped to one component, prefer using the relevant prefix. Split commits when that improves clarity.
How to generate a changelog entry¶
Changelog entries are generated automatically right before the release of a new CTA version. A changelog entry is generated for every commit containing a Changelog:
trailer.
By default, you will see the following squash commit template in the GitLab UI:
%{title} (%{reference})
%{issues}
Changelog: addition/fix/change/deprecation/removal/security/performance/other -- remove this line if no changelog entry required
The last line is what determines if and where your commit ends up in the changelog. The following options are available:
addition
: for any new features addedfix
: for any bug fixeschange
: for any features that changed in functionalitydeprecation
: for any changes that deprecated (but did not remove) functionalityremoval
: for any changes that removed functionalitysecurity
: for any serious security fixes/changesperformance
: for any changes improving performance but not changing functionalityother
: for any changes that do not fall in the categories above
As described in the section What warrants a changelog entry
, not every commit should end up in the changelog. If your commit should not end up in the changelog, simply remove the last line; ensure that the Changelog:
trailer is not present.
Squashing Commits¶
While working on the code, you will be making multiple commits. Before this is merged into main
, these must be squashed into a single well-defined commit. Commits should be focussed; they should be adding and/or fixing one particular thing. This is important for other people inspecting the code or in case of e.g. a rollback. If your commit cannot be properly described by a single hypothetical changelog entry, then you should split up your MR.
If you want to squash all commits into a single commit, then you are highly advised to do this using the GitLab UI in the MR itself as this makes it extremely easy and automatically provides the correct squash commit template. Any commit that ends up in main
must follow squash commit template in the GitLab UI.
Adding an additional description to the commit is optional.
Manually squashing commits (not recommended)¶
If you want to manually squash commits, make sure you follow the exact same squash commit template as on GitLab.
Start by identifying the commits in your history:
> git log --oneline
330ccecf5 (HEAD -> main) Add semi-related feature 2
673d6fc95 Last commit, trust me
3f52ec780 Oh no I forgot something
7b505e72d Fix bug to get feature 1 working
25d2b370b Add feature 1
50e19209c (origin/main, origin/HEAD) Changelog versioning update
Identify until which commit you want to squash. In this case, the previous commit from the main branch was 50e19209c
(Changelog versioning update
). Now you can start an interactive rebase from this commit, which will allow you squash commits starting at this commit. Note that the commit you provide to the rebase
command is exclusive; this means that this commit will not be part of the rebase.
git rebase -i 50e19209c
Will open the following in your configured git text editor:
pick 25d2b370b Add feature 1
pick 7b505e72d Fix bug to get feature 1 working
pick 3f52ec780 Oh no I forgot something
pick 673d6fc95 Last commit, trust me
pick 330ccecf5 Add semi-related feature 2
These commits are ordered in reverse chronological order. For the purpose of squashing, we can mark the commits as one of the following:
pick
keeps the commit as is.squash
combines this commit with the previous one and lets you edit.fixup
combines this commit with the previous one but discards its commit message (useful if you want to simplify the commit history).
If you want to squash everything into a single commit, we can mark the commits as follows:
pick 25d2b370b Add feature 1
squash 7b505e72d Fix bug to get feature 1 working
squash 3f52ec780 Oh no I forgot something
squash 673d6fc95 Last commit, trust me
squash 330ccecf5 Add semi-related feature 2
After this, you will be able to edit the commit message of the new squashed commit.
If you want to squash this commit history into two separate commits, you can do e.g.:
pick 25d2b370b Add feature 1
squash 7b505e72d Fix bug to get feature 1 working
squash 3f52ec780 Oh no I forgot something
pick 673d6fc95 Last commit, trust me
squash 330ccecf5 Add semi-related feature 2
In this case, we will get two separate commits. The first commit combining 25d2b370b
-3f52ec780
and the second commit combining 673d6fc95
-330ccecf5
.
Once again, you will be able to edit the commit message of both of these new squashed commits. When doing so, ensure that you follow the Git commit template specified above.
Once done with the squashing, we have rewritten history (and they said it couldn't be done). This has the unfortunate side-effect that we cannot directly push to our branch. Instead, we will have to force push (not in the jedi way) to the branch that we are currently on. Force pushing is always dangerous as it overwrites the remote history, potentially erasing commits. As such, if you screw it up there is no way to get it back without a backup. As such, it does not hurt to have a quick backup somewhere of the branch in case something goes wrong. With all these warnings out of the way, execute the following to update the remote branch with the new git history:
git push --force-with-lease
Note that we use --force-with-lease
instead of --force
. This prevents scenarios in which you are overwriting commits that are on the remote, but were not on your local branch (e.g. because someone else added them and you did not pull the changes). See it as a small extra safeguard.
At this point the added commits should be clean, compliant with the required format and ready to be merged into the main
branch.