Before making any merge request, please make sure you have run the appropriate tests for the release (git branch) which you are targetting. For instance, for the tier-0 release you need to respect the frozen tier 0 policy and run the RunTier0Tests.py
script. For all releases you should explain what tests you have run when making the merge request (see below).
When you push to GitLab, you’ll get several prompts to create a merge
request, both at the command line and when you open the project
browser. You should ignore these: GitLab will very slow because it
will assume you want to merge with main
and find a huge number of
changes. Fortunately there’s a shortcut to avoid this! You can instead
select the “merge requests” button on the left tab:
From here you can create a new merge request
A page now opens that allows you to define the merge request. There are a few points you should pay attention to.
Target branch
is correct. This is set by default
to main
, but if your merge is not for the Athena development
branch you must change it. (A useful cross check is to
see how many Changes
are in your merge request - if it’s far more
than you expected then the target branch is probably wrong.)Title
and Description
are taken from the last commit message
of the topic branch. Please edit these to ensure that these fields
correctly describe the merge to the person who will review the code
Title
with a topic, e.g. MyPackage: ...
if your changes only affect one package.(Here we show a different merge request, but the principle is all the same.)
Before submitting the merge request, you may want to briefly consult the checklist for the review shifters. Make sure that you have taken into account all points to ensure a fast and smooth review process.
Once you are happy with the merge request just press the Submit merge request
button.
If you are just doing the tutorial as an exercise then please do not actually submit a dummy merge request, just stop here!
After this happens the merge request page on the main repository will open up:
(Note that that screen shot was taken after the continuous integration system had run - initially many of the fields and labels will be blank.)
By default, all MRs are squashed. You can read more about it here, but the basic idea is the commits you made when developing the MR are compressed into one final commit (and you will might want to edit this commit message, to make sure it is informative and complete). If your commits are well factorised and not too numerous, then you can choose to deselect the ‘squash’ option. However, the final decision about whether to squash or not is the release coordinator, whose job it is to keep the repository commit history clean and comprehensible.
ATLAS uses GitLab labels to guide the process of reviewing and handling merge requests. As the developer you have input into this process - by setting appropriate labels on your request you ensure that it is handled correctly by the reviewers and release coordinators.
Note that most labels will just be set automatically by the continuous integration system. Here we list the ones that you might need to set by hand:
urgent | This merge request fixes something that is high priority, it will jump the queue for review | |
frozen-tier0-violating | This merge request changes the output from Tier-0 and as such requires special attention from PROC (see the twiki for FT0 policy) | |
bugfix | Informs the reviewers and release coordinators that this request fixes an undesired behaviour which may need to be followed up on | |
full-build | The CI machine will clear its CMake build area before and after compiling the MR. This makes the CI take a lot longer to run, but is needed to successfully build some low-level code changes. | |
full-unit-tests | Run the full suite of unit tests for all packages, not just the packages touched by the merge request. | |
full-integration-tests | Run the full suite of integration tests, rather than the subset determined as required for the merge request via a heuristic matching. |
The production branch (currently upstream/24.0
) is regularly merged into the main branch
(upstream/main
) by the release coordinators. They may ask for your help if your merge request
to upstream/24.0
is found to conflict with changes which previously went only into upstream/main
.
For changes which are needed in other branches a cherry-pick operations should be used to make duplicates of the
merge request for the other branch(es). This may be used, for example, to back-port a bug fix to release 21.X branches,
or any other legacy branches. This can be achieved via alsoTargetting:XX
labels, where XX
denotes a target branch.
Don’t use
alsoTargetting
labels to cherry-pick between the production branch and the main branch. The release coordinator will regularly merge the production branch into the main branch.
ATLAS runs a script that takes accepted
merge requests and tries to cherry-pick these changes onto the target branches specified by the labels.
If the merge would succeed without a conflict then a new
merge request is created (triggering the normal CI and review
process) and the label sweep:done
is added to the MR.
If any requested branches would not then the label sweep:failed
is also added and the change should be ported manually.
GitLab provides a Cherry-pick button on merge requests which have been merged. Clicking this is equivalent to adding a sweep label and using the sweeping script.
If you need to update a merge request (for example in response to a request from the review shifter), simply push to the same branch again after editing (and testing) the files.
This question has the same answer as “how long is a piece of string?”… it depends on the circumstances.
However, very long merge requests that touch dozens of files and thousands of lines are in general a problem for the following reasons:
In general, MRs should aim to be rather “atomic”, that is, they should only touch a small number of tools/algorithms or configuration blocks - one tool/algorithm per MR might even be preferred. This makes a given MR more digestible and easier to describe and review. Developments that involve touching many different algorithms/tools/config blocks should make use of JIRA tickets (or an epic with associated tickets) which enable the overall development to be described on the ticket and then each MR linked to it.
Developers should not be worried about submitting temporarily “dead code”, that is, new tools/algorithms that won’t be put to use immediately after submission. One should simply write in the MR description something along the lines of
the code in this MR will not be used until X, Y and Z are also implemented as described in the linked JIRA ticket - so this MR will not affect any production or validation workflows until these other steps are completed
These guidelines do not apply to cases where the same small change has been applied in bulk to many files (for example changes to many CMake files at once or some small code quality update). It will be immediately obvious to the shifter that the same change is being applied in many places and the review will be straightforward.
On the other hand, long MRs that result from porting code from (e.g.) release 21 to main should still be broken up into smaller parts, even if the code was reviewed in r21.
Sometimes it is convenient when collaborating on a development to have all of the code changes in a single large draft MR. This is fine. Once the group of people working on the development have converged and are ready to submit it for review, they should then agree how best to split it up, submit new non-draft MRs, and close the big draft MR. It is very important that such large work-in-progress MRs are kept as draft to avoid premature reviews by shifters.