-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Display draft releases #1854
Display draft releases #1854
Conversation
Integration tests are welcome. |
} | ||
r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas()) | ||
releasesToDisplay = append(releasesToDisplay, r) |
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 wouldn't be "faster" using the index of the for loop and directly assigning releasesToDisplay[index] ?
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.
We sometimes continue
(line 88)
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.
ok but we don't exceed len(releases)
+ if we skip we should not use pager := paginater.New(len(releases), limit, page, 5)
but len(releasesToDisplay)
routers/repo/release.go
Outdated
@@ -79,7 +79,7 @@ func Releases(ctx *context.Context) { | |||
|
|||
// Temporary cache commits count of used branches to speed up. | |||
countCache := make(map[string]int64) | |||
var cacheUsers = make(map[int64]*models.User) | |||
cacheUsers := map[int64]*models.User{ctx.User.ID: ctx.User} |
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.
Good idea
Except my little proposition for optimization and that we need more testing generally. This PR LGTM. |
@lunny Added an integration test |
LGTM |
Fixes #1387. Previously, we only displayed releases whose tag name actually existed in the repository, which is why drafts did not show up.