Skip to content

Commit

Permalink
Fixes for Parallel Working Branch strategy (#592)
Browse files Browse the repository at this point in the history
- Fix retry for finalize to not create multiple PRs
- Check for diff between branches before creating PR
  • Loading branch information
nid90 authored Dec 29, 2023
1 parent e913224 commit 577d4cc
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 10 deletions.
6 changes: 3 additions & 3 deletions app/controllers/releases_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ def set_release
end

def set_pull_requests
@pre_release_prs = @release.pull_requests.pre_release
@post_release_prs = @release.pull_requests.post_release
@ongoing_open_release_prs = @release.pull_requests.ongoing.open
@pre_release_prs = @release.pre_release_prs
@post_release_prs = @release.post_release_prs
@ongoing_open_release_prs = @release.backmerge_prs.open
end

def set_commits
Expand Down
11 changes: 11 additions & 0 deletions app/libs/installations/github/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,8 @@ def branch_exists?(repo, branch_name)
end

def create_pr!(repo, to, from, title, body, transforms)
raise Installations::Errors::PullRequestWithoutCommits unless diff?(repo, to, from)

execute do
@client
.create_pull_request(repo, to, from, title, body)
Expand Down Expand Up @@ -258,6 +260,15 @@ def commits_between(repo, from_branch, to_branch, transforms)
end
end

def diff?(repo, from_branch, to_branch)
execute do
@client
.compare(repo, from_branch, to_branch)
.dig(:files)
.present?
end
end

def head(repo, working_branch_name, sha_only: true, commit_transforms: nil)
raise ArgumentError, "transforms must be supplied when querying head object" if !sha_only && !commit_transforms

Expand Down
3 changes: 2 additions & 1 deletion app/libs/triggers/post_release/parallel_branches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ def create_and_merge_pr
to_branch_ref: working_branch,
from_branch_ref: release_branch,
title: pr_title,
description: pr_description
description: pr_description,
existing_pr: release.pull_requests.post_release.first
).then do |value|
stamp_pr_success
GitHub::Result.new { value }
Expand Down
1 change: 1 addition & 0 deletions app/libs/triggers/release.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def call
raise ReleaseAlreadyInProgress.new("No more releases can be started until the ongoing release is finished!") if train.upcoming_release.present?
raise UpcomingReleaseNotAllowed.new("Upcoming releases are not allowed for your train.") if train.ongoing_release.present? && !train.upcoming_release_startable?
raise NothingToRelease.new("No diff since last release") if regular_release? && !train.diff_since_last_release?
raise NothingToRelease.new("No diff between working and release branch") if train.parallel_working_branch? && !train.diff_for_release?
train.activate! unless train.active?
create_release
train.create_webhook!
Expand Down
4 changes: 4 additions & 0 deletions app/models/github_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ def commit_log(from_branch, to_branch)
installation.commits_between(code_repository_name, from_branch, to_branch, COMMITS_TRANSFORMATIONS)
end

def diff_between?(from_branch, to_branch)
installation.diff?(code_repository_name, from_branch, to_branch)
end

def public_icon_img
PUBLIC_ICON
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/gitlab_integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ def commit_log(from_branch, to_branch)
with_api_retries { installation.commits_between(code_repository_name, from_branch, to_branch, COMMITS_BETWEEN_TRANSFORMATIONS) }
end

def diff_between?(from_branch, to_branch)
with_api_retries { installation.diff?(code_repository_name, from_branch, to_branch) }
end

def create_patch_pr!(to_branch, patch_branch, commit_hash, pr_title_prefix)
# FIXME
{}.merge_if_present(source: :gitlab)
Expand Down
10 changes: 9 additions & 1 deletion app/models/release.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class Release < ApplicationRecord
attr_accessor :has_major_bump, :force_finalize, :hotfix_platform, :custom_version

delegate :versioning_strategy, to: :train
delegate :app, :pre_release_prs?, :vcs_provider, :release_platforms, :notify!, :continuous_backmerge?, to: :train
delegate :app, :vcs_provider, :release_platforms, :notify!, :continuous_backmerge?, to: :train
delegate :platform, to: :app

def self.pending_release?
Expand All @@ -161,6 +161,14 @@ def backmerge_prs
pull_requests.ongoing
end

def post_release_prs
pull_requests.post_release
end

def pre_release_prs
pull_requests.pre_release
end

def duration
return unless finished?
ActiveSupport::Duration.build(completed_at - scheduled_at)
Expand Down
11 changes: 8 additions & 3 deletions app/models/train.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,14 @@ def last_run_at
end

def diff_since_last_release?
return vcs_provider.commit_log(ongoing_release.first_commit.commit_hash, working_branch).any? if ongoing_release
return vcs_provider.diff_between?(ongoing_release.first_commit.commit_hash, working_branch) if ongoing_release
return true if last_finished_release.blank?
vcs_provider.commit_log(last_finished_release.tag_name || last_finished_release.last_commit.commit_hash, working_branch).any?
vcs_provider.diff_between?(last_finished_release.tag_name || last_finished_release.last_commit.commit_hash, working_branch)
end

def diff_for_release?
return false unless parallel_working_branch?
vcs_provider.diff_between?(release_branch, working_branch)
end

def create_webhook!
Expand Down Expand Up @@ -328,7 +333,7 @@ def open_active_prs_for(branch_name)
PullRequest.open.where(release_id: active_runs.ids, head_ref: branch_name)
end

def pre_release_prs?
def parallel_working_branch?
branching_strategy == "parallel_working"
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/releases/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
<div>
<%= render partial: "shared/live_release/kick_off", locals: { release: @release, release_train: @train } %>

<% if @release.pre_release_prs? %>
<% if @pre_release_prs.present? %>
<%= render partial: "shared/live_release/separator", locals: { margin_only: true } %>
<%= render partial: "shared/live_release/pre_release_prs", locals: { pre_release_prs: @pre_release_prs } %>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion spec/libs/triggers/release_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
allow_any_instance_of(GooglePlayStoreIntegration).to receive(:draft_check?).and_return(false)
allow(Installations::Github::Jwt).to receive(:new).and_return(github_jwt_double)
allow(Installations::Github::Api).to receive(:new).and_return(github_api_double)
allow(github_api_double).to receive(:commits_between).and_return([1])
allow(github_api_double).to receive(:diff?).and_return(true)
end

it "creates a new release" do
Expand Down

0 comments on commit 577d4cc

Please sign in to comment.