Fix release display and correct paging #2080
Conversation
|
Continuation to #2035 |
tboerger
added
the
lgtm/need 2
lafriks
force-pushed
the
fix/release_listing
branch
from
b0236c7
to
afc8804
Compare
| } | ||
|
|
||
| // GetReleaseCountByRepoID returns the count of releases of repository | ||
| func GetReleaseCountByRepoID(repoID int64, includeDrafts bool) (int64, error) { | ||
| func (opts *FindReleasesOptions) toConds(repoID int64) builder.Cond { |
There was a problem hiding this comment.
Why not let repoID as a field of FindReleasesOptions?
There was a problem hiding this comment.
At first I did that but than it looked strange as func name is GetReleasesByRepoID. And also repoID is mandatory condition
There was a problem hiding this comment.
I think FindReleases and CountReleases maybe a new name?
There was a problem hiding this comment.
But repoID is still allways required field and that will not easy be understandable by function signature
| // GetReleasesByRepoIDAndNames returns a list of releases of repository according repoID and tagNames. | ||
| func GetReleasesByRepoIDAndNames(repoID int64, tagNames []string) (rels []*Release, err error) { | ||
| // GetReleasesByRepoID returns a list of releases of repository. | ||
| func GetReleasesByRepoID(repoID int64, opts FindReleasesOptions, page, pageSize int) (rels []*Release, err error) { |
There was a problem hiding this comment.
As above and page and pageSize is also could be fields of FindReleasesOptions
There was a problem hiding this comment.
Because it's not really related to conditions and toConds can't use them anyway
|
LGTM |
tboerger
added
lgtm/need 1
|
LGTM |
tboerger
added
lgtm/done
Fixes #1863