diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-03-07 09:01:26 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-03-07 09:01:26 +0000 |
commit | d43c778402b481569cb8b2fffed45a7c939e0c75 (patch) | |
tree | 463ca3d213963d98ecc6f8ca4b8b60cc52195163 /spec | |
parent | be1ae2d660ad98cf6a44cefa3148f9c48b66806f (diff) | |
parent | 543845f7efe0b70926ea699eaf1e413fa878b285 (diff) | |
download | gitlab-ce-d43c778402b481569cb8b2fffed45a7c939e0c75.tar.gz |
Merge branch 'indicate-mr-diverged-from-target' into 'master'
Indicate when an MR diverged from the target branch
This adds an indicator to the "Merge MR" box, to tell if and how much an MR diverged from its target branch.
For instance, consider an MR to merge the branch `feature` into `master`. Some other commits were added to `master` since `feature` was created, and the two branches diverged.
```text
o master
|
o o feature
| |
o o
| /
o
```
In this case, there will be a label in the MR Merge box stating:
> This MR is by 3 commits behind the target branch `master`.
## Screenshots
### The branch diverged from the target (UI Proposal)
![UI_suggestion_1](/uploads/cd5bee3959e68026ec7d5097259d53f4/UI_suggestion_1.png)
### The branch diverged from the target (alternative UI Proposal)
![UI_suggestion_2](/uploads/f36977101b59a610850e129837dfbc83/UI_suggestion_2.png)
## How is this useful?
- In a _rebase-workflow_ (MR are preferably rebased before being merged), the reviewer wants to know if an MR is rebased on the target branch before merging it.
_With this indicator, the reviewer knows immediately if the branch is rebased, or if she needs to ask the committer to rebase its branch._
<br>
- To keep the git history readable, a team prefers to avoid merging branches that really lag a lot behind the target branch. Merging an MR that is 10 commits behind is fine, but 200 is too much.
_With this indicator, the reviewer can see on the MR page if the branch is really far behind the target – or only a few commits behind._
## Open questions
We've been using this at @captaintrain for a few months now, and found it quite useful.
I guess the open-questions are mostly: what UI would be the more adequate? Any thoughts on this, on the general usefulness and/or on the code?
See merge request !2217
Diffstat (limited to 'spec')
-rw-r--r-- | spec/factories/merge_requests.rb | 10 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 64 |
2 files changed, 74 insertions, 0 deletions
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 00de7bb5294..ca1c636fce4 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -69,6 +69,16 @@ FactoryGirl.define do target_branch "master" end + trait :rebased do + source_branch "markdown" + target_branch "improve/awesome" + end + + trait :diverged do + source_branch "feature" + target_branch "master" + end + trait :merge_when_build_succeeds do merge_when_build_succeeds true merge_user author diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c51f34034d7..59c40922abb 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -274,6 +274,70 @@ describe MergeRequest, models: true do end end + describe '#diverged_commits_count' do + let(:project) { create(:project) } + let(:fork_project) { create(:project, forked_from_project: project) } + + context 'diverged on same repository' do + subject(:merge_request_with_divergence) { create(:merge_request, :diverged, source_project: project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(5) + end + end + + context 'diverged on fork' do + subject(:merge_request_fork_with_divergence) { create(:merge_request, :diverged, source_project: fork_project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(5) + end + end + + context 'rebased on fork' do + subject(:merge_request_rebased) { create(:merge_request, :rebased, source_project: fork_project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(0) + end + end + + describe 'caching' do + before(:example) do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + end + + it 'caches the output' do + expect(subject).to receive(:compute_diverged_commits_count). + once. + and_return(2) + + subject.diverged_commits_count + subject.diverged_commits_count + end + + it 'invalidates the cache when the source sha changes' do + expect(subject).to receive(:compute_diverged_commits_count). + twice. + and_return(2) + + subject.diverged_commits_count + allow(subject).to receive(:source_sha).and_return('123abc') + subject.diverged_commits_count + end + + it 'invalidates the cache when the target sha changes' do + expect(subject).to receive(:compute_diverged_commits_count). + twice. + and_return(2) + + subject.diverged_commits_count + allow(subject).to receive(:target_sha).and_return('123abc') + subject.diverged_commits_count + end + end + end + it_behaves_like 'an editable mentionable' do subject { create(:merge_request) } |