Skip to content

Commit

Permalink
pull: add an option to use rebase for merging pull requests
Browse files Browse the repository at this point in the history
For DVCS, either merge or rebase works for getting new code in a pull
request in the main branch.

The rebase workflow produces a linear history which is cleaner, and
more bisect-able.

This commit adds a repo-level option to enable the rebase workflow. Once
enabled, "Merge Pull Request" will be replaced by
"Rebase and Merge Pull Request" which does exactly what the user wants.
It's unlikely a project wants a mixed-use of both rebase and merge
workflows, therefore the feature is not implemented as a drop-down
button like what GitHub does
(https://github.com/blog/2243-rebase-and-merge-pull-requests).
  • Loading branch information
mharinder authored and unknwon committed Nov 16, 2017
1 parent dbe6de3 commit 5cd1fde
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 27 deletions.
2 changes: 2 additions & 0 deletions conf/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,7 @@ pulls.can_auto_merge_desc = This pull request can be merged automatically.
pulls.cannot_auto_merge_desc = This pull request can't be merged automatically because there are conflicts.
pulls.cannot_auto_merge_helper = Please merge manually in order to resolve the conflicts.
pulls.merge_pull_request = Merge Pull Request
pulls.rebase_merge_pull_request = Rebase and Merge Pull Request
pulls.open_unmerged_pull_exists = `You can't perform reopen operation because there is already an open pull request (#%d) from same repository with same merge information and is waiting for merging.`
pulls.delete_branch = Delete Branch
pulls.delete_branch_has_new_commits = Branch cannot be deleted because it has new commits after mergence.
Expand Down Expand Up @@ -737,6 +738,7 @@ settings.tracker_issue_style.numeric = Numeric
settings.tracker_issue_style.alphanumeric = Alphanumeric
settings.tracker_url_format_desc = You can use placeholder <code>{user} {repo} {index}</code> for user name, repository name and issue index.
settings.pulls_desc = Enable pull requests to accept public contributions
settings.use_rebase_desc = Use rebase to merge pull requests
settings.danger_zone = Danger Zone
settings.cannot_fork_to_same_owner = You cannot fork a repository to its original owner.
settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name.
Expand Down
35 changes: 22 additions & 13 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,31 +233,40 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}

// Merge commits.
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath),
"git", "fetch", "head_repo"); err != nil {
return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}

if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath),
"git", "merge", "--no-ff", "--no-commit", "head_repo/"+pr.HeadBranch); err != nil {
return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr)
}
if (pr.BaseRepo.PullUseRebase) {
// Rebase.
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Rebase (git rebase): %s", tmpBasePath),
"git", "rebase", "-q", pr.BaseBranch, "head_repo/"+pr.HeadBranch); err != nil {
return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}
} else {
// Merge commits.
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath),
"git", "merge", "--no-ff", "--no-commit", "head_repo/"+pr.HeadBranch); err != nil {
return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr)
}

sig := doer.NewGitSig()
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
"git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),
"-m", fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch)); err != nil {
return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, stderr)
sig := doer.NewGitSig()
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
"git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),
"-m", fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch)); err != nil {
return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, stderr)
}
}

// Push back to upstream.
if _, stderr, err = process.ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git push): %s", tmpBasePath),
"git", "push", baseGitRepo.Path, pr.BaseBranch); err != nil {
"git", "push", "head_repo", "HEAD:"+pr.BaseBranch); err != nil {
return fmt.Errorf("git push: %s", stderr)
}

Expand Down
1 change: 1 addition & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ type Repository struct {
ExternalTrackerStyle string
ExternalMetas map[string]string `xorm:"-"`
EnablePulls bool `xorm:"NOT NULL DEFAULT true"`
PullUseRebase bool `xorm:"NOT NULL DEFAULT false"`

IsFork bool `xorm:"NOT NULL DEFAULT false"`
ForkID int64
Expand Down
24 changes: 12 additions & 12 deletions pkg/bindata/bindata.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pkg/form/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type RepoSetting struct {
TrackerURLFormat string
TrackerIssueStyle string
EnablePulls bool
PullUseRebase bool
}

func (f *RepoSetting) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors {
Expand Down
1 change: 1 addition & 0 deletions routes/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ func SettingsPost(c *context.Context, f form.RepoSetting) {
repo.ExternalTrackerFormat = f.TrackerURLFormat
repo.ExternalTrackerStyle = f.TrackerIssueStyle
repo.EnablePulls = f.EnablePulls
repo.PullUseRebase = f.PullUseRebase

if err := models.UpdateRepository(repo, false); err != nil {
c.ServerError("UpdateRepository", err)
Expand Down
6 changes: 5 additions & 1 deletion templates/repo/issue/view_content.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,11 @@
<form class="ui form" action="{{.Link}}/merge" method="post">
{{.CSRFTokenHTML}}
<button class="ui green button">
<span class="octicon octicon-git-merge"></span> {{$.i18n.Tr "repo.pulls.merge_pull_request"}}
{{if .Issue.Repo.PullUseRebase }}
<span class="octicon octicon-git-pull-request"></span> {{$.i18n.Tr "repo.pulls.rebase_merge_pull_request"}}
{{else}}
<span class="octicon octicon-git-merge"></span> {{$.i18n.Tr "repo.pulls.merge_pull_request"}}
{{end}}
</button>
</form>
</div>
Expand Down
8 changes: 7 additions & 1 deletion templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,16 @@
<div class="inline field">
<label>{{.i18n.Tr "repo.pulls"}}</label>
<div class="ui checkbox">
<input name="enable_pulls" type="checkbox" {{if .Repository.EnablePulls}}checked{{end}}>
<input class="enable-system" name="enable_pulls" type="checkbox" data-target="#pull_box" {{if .Repository.EnablePulls}}checked{{end}}>
<label>{{.i18n.Tr "repo.settings.pulls_desc"}}</label>
</div>
</div>
<div class="ui segment field {{if not .Repository.EnablePulls}}disabled{{end}}" id="pull_box">
<div class="ui checkbox">
<input name="pull_use_rebase" type="checkbox" {{if .Repository.PullUseRebase}}checked{{end}}>
<label>{{.i18n.Tr "repo.settings.use_rebase_desc"}}</label>
</div>
</div>
{{end}}

<div class="field">
Expand Down

0 comments on commit 5cd1fde

Please sign in to comment.