-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Branch struct in package modules/git #33980
base: main
Are you sure you want to change the base?
Conversation
@@ -169,13 +169,6 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u | |||
} | |||
|
|||
if pr.Flow == issues_model.PullRequestFlowAGit { | |||
gitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 146 has created gitRepo
.
d537b13
to
c46d935
Compare
@@ -68,34 +56,11 @@ func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { | |||
} | |||
|
|||
// GetBranch returns a branch by it's name | |||
func (repo *Repository) GetBranch(branch string) (*Branch, error) { | |||
func (repo *Repository) GetBranch(branch string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function can be replaced by repo.IsBranchExist
, so this function can be removed?
// GetHEADBranch returns corresponding branch of HEAD. | ||
func (repo *Repository) GetHEADBranch() (*Branch, error) { | ||
func (repo *Repository) GetHEADBranch() (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetHEADBranchName
maybe better, as this can be easily considered that the return is a string.
@@ -28,8 +28,8 @@ import ( | |||
// Optional - Merger | |||
func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User) *api.PullRequest { | |||
var ( | |||
baseBranch *git.Branch | |||
headBranch *git.Branch | |||
baseBranch string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
baseBranchName & headBranchName
The
Branch
struct inmodules/git
package is unnecessary. We can just use astring
to represent a branch