-
-
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 "refs/*" string usage by using constants #17784
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,6 +13,7 @@ import ( | |||||
"code.gitea.io/gitea/models/db" | ||||||
"code.gitea.io/gitea/models/unit" | ||||||
user_model "code.gitea.io/gitea/models/user" | ||||||
"code.gitea.io/gitea/modules/git" | ||||||
"code.gitea.io/gitea/modules/log" | ||||||
"code.gitea.io/gitea/modules/setting" | ||||||
"code.gitea.io/gitea/modules/timeutil" | ||||||
|
@@ -349,7 +350,7 @@ func (pr *PullRequest) GetDefaultSquashMessage() string { | |||||
|
||||||
// GetGitRefName returns git ref for hidden pull request branch | ||||||
func (pr *PullRequest) GetGitRefName() string { | ||||||
return fmt.Sprintf("refs/pull/%d/head", pr.Index) | ||||||
return fmt.Sprintf(git.PullPrefix+"%d/head", pr.Index) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason why this isn't
Suggested change
|
||||||
} | ||||||
|
||||||
// IsChecking returns true if this pull request is still checking conflict. | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -79,7 +79,7 @@ func ToAPIPullRequest(pr *models.PullRequest, doer *user_model.User) *api.PullRe | |||||
}, | ||||||
Head: &api.PRBranchInfo{ | ||||||
Name: pr.HeadBranch, | ||||||
Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index), | ||||||
Ref: fmt.Sprintf(git.PullPrefix+"%d/head", pr.Index), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you can send another PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
RepoID: -1, | ||||||
}, | ||||||
} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,15 @@ package git | |
|
||
import "strings" | ||
|
||
const ( | ||
// RemotePrefix is the base directory of the remotes information of git. | ||
RemotePrefix = "refs/remotes/" | ||
// PullPrefix is the base directory of the pull information of git. | ||
PullPrefix = "refs/pull/" | ||
|
||
pullLen = len(PullPrefix) | ||
) | ||
|
||
// Reference represents a Git ref. | ||
type Reference struct { | ||
Name string | ||
|
@@ -24,17 +33,17 @@ func (ref *Reference) ShortName() string { | |
if ref == nil { | ||
return "" | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/heads/") { | ||
return ref.Name[11:] | ||
if strings.HasPrefix(ref.Name, BranchPrefix) { | ||
return strings.TrimPrefix(ref.Name, BranchPrefix) | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/tags/") { | ||
return ref.Name[10:] | ||
if strings.HasPrefix(ref.Name, TagPrefix) { | ||
return strings.TrimPrefix(ref.Name, TagPrefix) | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/remotes/") { | ||
return ref.Name[13:] | ||
if strings.HasPrefix(ref.Name, RemotePrefix) { | ||
return strings.TrimPrefix(ref.Name, RemotePrefix) | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/pull/") && strings.IndexByte(ref.Name[10:], '/') > -1 { | ||
return ref.Name[10 : strings.IndexByte(ref.Name[10:], '/')+10] | ||
if strings.HasPrefix(ref.Name, PullPrefix) && strings.IndexByte(ref.Name[pullLen:], '/') > -1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bit unfair that only PullPrefix gets a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I felt this would be overhead for the others. But consistency is important too. |
||
return ref.Name[pullLen : strings.IndexByte(ref.Name[pullLen:], '/')+pullLen] | ||
} | ||
|
||
return ref.Name | ||
|
@@ -45,16 +54,16 @@ func (ref *Reference) RefGroup() string { | |
if ref == nil { | ||
return "" | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/heads/") { | ||
if strings.HasPrefix(ref.Name, BranchPrefix) { | ||
return "heads" | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/tags/") { | ||
if strings.HasPrefix(ref.Name, TagPrefix) { | ||
return "tags" | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/remotes/") { | ||
if strings.HasPrefix(ref.Name, RemotePrefix) { | ||
return "remotes" | ||
} | ||
if strings.HasPrefix(ref.Name, "refs/pull/") && strings.IndexByte(ref.Name[10:], '/') > -1 { | ||
if strings.HasPrefix(ref.Name, PullPrefix) && strings.IndexByte(ref.Name[pullLen:], '/') > -1 { | ||
return "pull" | ||
} | ||
return "" | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -8,6 +8,8 @@ package migration | |||||
import ( | ||||||
"fmt" | ||||||
"time" | ||||||
|
||||||
"code.gitea.io/gitea/modules/git" | ||||||
) | ||||||
|
||||||
// PullRequest defines a standard pull request information | ||||||
|
@@ -43,7 +45,7 @@ func (p *PullRequest) IsForkPullRequest() bool { | |||||
|
||||||
// GetGitRefName returns pull request relative path to head | ||||||
func (p PullRequest) GetGitRefName() string { | ||||||
return fmt.Sprintf("refs/pull/%d/head", p.Number) | ||||||
return fmt.Sprintf(git.PullPrefix+"%d/head", p.Number) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue:
Suggested change
|
||||||
} | ||||||
|
||||||
// PullRequestBranch represents a pull request branch | ||||||
|
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.
Is there a reason why this isn't
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.
No specific reason other than I didn't thought of it. Your proposal is indeed more elegant.