-
-
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
Don't create duplicated functions for code repositories and wiki repositories #33924
Changes from all commits
6c2806f
da76fc1
76e742a
ffac9aa
634b489
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 |
---|---|---|
|
@@ -215,12 +215,24 @@ func init() { | |
db.RegisterModel(new(Repository)) | ||
} | ||
|
||
func (repo *Repository) GetName() string { | ||
return repo.Name | ||
func RelativePath(ownerName, repoName string) string { | ||
return strings.ToLower(ownerName) + "/" + strings.ToLower(repoName) + ".git" | ||
} | ||
|
||
func (repo *Repository) GetOwnerName() string { | ||
return repo.OwnerName | ||
// RelativePath should be an unix style path like username/reponame.git | ||
func (repo *Repository) RelativePath() string { | ||
return RelativePath(repo.OwnerName, repo.Name) | ||
} | ||
|
||
type StorageRepo string | ||
|
||
// RelativePath should be an unix style path like username/reponame.git | ||
func (sr StorageRepo) RelativePath() string { | ||
return string(sr) | ||
} | ||
|
||
func (repo *Repository) WikiStorageRepo() StorageRepo { | ||
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.
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. It's a storage repository interface, not only path. |
||
return StorageRepo(strings.ToLower(repo.OwnerName) + "/" + strings.ToLower(repo.Name) + ".wiki.git") | ||
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. We could check here that we don't already have a Wiki repo: 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. I want to add a comment here: currently, Repository refers to the entire concept of a repository. I don’t believe it can be interpreted to mean a wiki repository specifically. |
||
} | ||
|
||
// SanitizedOriginalURL returns a sanitized OriginalURL | ||
|
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.
RepoPath
?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.
may
RepoPath
can cause confusion with repo url path? maybeStoragePath
?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.
There is already a complex Storage system in Gitea.
TBH any new variable named "StorageXxx" doesn't look good to me (but I won't say block)
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, I would say it does not:
After all, we are on the
models
layer here.As such, it is far more likely that we are talking about how the repo is stored compared to how it is accessed.
If it had something to do with URLs, we would need the URL somewhere in the name, since that stuff is normally handled by
routers
andservices
.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.
It maybe refactored to add more interface functions rather than repo path, so I don't think it's a good name as
RepoPath
. And this is an implementation ofgitrepo.Repository
.