Skip to content
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

Update tool dependencies, lock govulncheck and actionlint #25655

Merged
merged 16 commits into from
Jul 9, 2023
Merged
Prev Previous commit
Next Next commit
fix
  • Loading branch information
wxiaoguang committed Jul 8, 2023
commit 0cef69b32323770d04a85fccae833a15c9e181a1
10 changes: 5 additions & 5 deletions models/activities/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func getIssueNotification(ctx context.Context, userID, issueID int64) (*Notifica
// NotificationsForUser returns notifications for a given user and status
func NotificationsForUser(ctx context.Context, user *user_model.User, statuses []NotificationStatus, page, perPage int) (notifications NotificationList, err error) {
if len(statuses) == 0 {
return
return nil, nil
}

sess := db.GetEngine(ctx).
Expand Down Expand Up @@ -372,16 +372,16 @@ func CountUnread(ctx context.Context, userID int64) int64 {
// LoadAttributes load Repo Issue User and Comment if not loaded
func (n *Notification) LoadAttributes(ctx context.Context) (err error) {
if err = n.loadRepo(ctx); err != nil {
return
return err
}
if err = n.loadIssue(ctx); err != nil {
return
return err
}
if err = n.loadUser(ctx); err != nil {
return
return err
}
if err = n.loadComment(ctx); err != nil {
return
return err
}
return err
Copy link
Member

@puni9869 puni9869 Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker.

https://github.com/go-gitea/gitea/pull/25655/files#diff-a9a3a131e7fcfb1641580afd9381afe56f44022d9a5e4ba651361f9230fa1694L386
what is the need of return err
we can put return nil

Sometime it hard to recall the Named return values in golang.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well we do report the error back so we need to retur err and not nil here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also a similar case of #25655 (comment)

Some code like return mode, fname, sha, n, err and return sha, typ, size, err are written as is intentionally, to avoid breaking.

The purpose of it is to avoid changing old code too much (which might break something or cause difficulty for reviewing).

In short: In Golang, return err is always right for named return values, but if you want to change it, the developers should always spend more time on thinking about "what value should be returned".

}
Expand Down
2 changes: 1 addition & 1 deletion models/asymkey/gpg_key_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func populateHash(hashFunc crypto.Hash, msg []byte) (hash.Hash, error) {
func readArmoredSign(r io.Reader) (body io.Reader, err error) {
block, err := armor.Decode(r)
if err != nil {
return
return nil, err
}
if block.Type != openpgp.SignatureType {
return nil, fmt.Errorf("expected '" + openpgp.SignatureType + "', got: " + block.Type)
Expand Down
10 changes: 5 additions & 5 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) {

err = json.Unmarshal([]byte(c.Content), &data)
if err != nil {
return
return err
}

c.IsForcePush = data.IsForcePush
Expand Down Expand Up @@ -925,7 +925,7 @@ func createIssueDependencyComment(ctx context.Context, doer *user_model.User, is
cType = CommentTypeRemoveDependency
}
if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

// Make two comments, one in each issue
Expand All @@ -937,7 +937,7 @@ func createIssueDependencyComment(ctx context.Context, doer *user_model.User, is
DependentIssueID: dependentIssue.ID,
}
if _, err = CreateComment(ctx, opts); err != nil {
return
return err
}

opts = &CreateCommentOptions{
Expand Down Expand Up @@ -1170,11 +1170,11 @@ func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullReques
return nil, fmt.Errorf("comment type %d cannot be used to create an auto merge comment", typ)
}
if err = pr.LoadIssue(ctx); err != nil {
return
return nil, err
}

if err = pr.LoadBaseRepo(ctx); err != nil {
return
return nil, err
}

comment, err = CreateComment(ctx, &CreateCommentOptions{
Expand Down
22 changes: 9 additions & 13 deletions models/issues/comment_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,42 +468,38 @@ func (comments CommentList) loadReviews(ctx context.Context) error {
// loadAttributes loads all attributes
func (comments CommentList) loadAttributes(ctx context.Context) (err error) {
if err = comments.LoadPosters(ctx); err != nil {
return
return err
}

if err = comments.loadLabels(ctx); err != nil {
return
return err
}

if err = comments.loadMilestones(ctx); err != nil {
return
return err
}

if err = comments.loadOldMilestones(ctx); err != nil {
return
return err
}

if err = comments.loadAssignees(ctx); err != nil {
return
return err
}

if err = comments.LoadAttachments(ctx); err != nil {
return
return err
}

if err = comments.loadReviews(ctx); err != nil {
return
return err
}

if err = comments.LoadIssues(ctx); err != nil {
return
}

if err = comments.loadDependentIssues(ctx); err != nil {
return
return err
}

return nil
return comments.loadDependentIssues(ctx)
}

// LoadAttributes loads attributes of the comments, except for attachments and
Expand Down
15 changes: 7 additions & 8 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ func (issue *Issue) LoadPoster(ctx context.Context) (err error) {
if !user_model.IsErrUserNotExist(err) {
return fmt.Errorf("getUserByID.(poster) [%d]: %w", issue.PosterID, err)
}
err = nil
return
return nil
}
}
return err
Expand Down Expand Up @@ -316,27 +315,27 @@ func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
// LoadAttributes loads the attribute of this issue.
func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

if err = issue.LoadPoster(ctx); err != nil {
return
return err
}

if err = issue.LoadLabels(ctx); err != nil {
return
return err
}

if err = issue.LoadMilestone(ctx); err != nil {
return
return err
}

if err = issue.LoadProject(ctx); err != nil {
return
return err
}

if err = issue.LoadAssignees(ctx); err != nil {
return
return err
}

if err = issue.LoadPullRequest(ctx); err != nil && !IsErrPullRequestNotExist(err) {
Expand Down
4 changes: 2 additions & 2 deletions models/issues/issue_label.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func newIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
}

if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

opts := &CreateCommentOptions{
Expand Down Expand Up @@ -168,7 +168,7 @@ func deleteIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *use
}

if err = issue.LoadRepo(ctx); err != nil {
return
return err
}

opts := &CreateCommentOptions{
Expand Down
34 changes: 17 additions & 17 deletions models/issues/issue_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,10 @@ func FindAndUpdateIssueMentions(ctx context.Context, issue *Issue, doer *user_mo
// don't have access to reading it. Teams are expanded into their users, but organizations are ignored.
func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *user_model.User, mentions []string) (users []*user_model.User, err error) {
if len(mentions) == 0 {
return
return nil, nil
}
if err = issue.LoadRepo(ctx); err != nil {
return
return nil, err
}

resolved := make(map[string]bool, 10)
Expand Down Expand Up @@ -635,7 +635,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
}
}
if len(mentionUsers) == 0 {
return
return users, err
}

if users == nil {
Expand Down Expand Up @@ -702,66 +702,66 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []
// Delete content histories
if _, err = sess.In("issue_id", deleteCond).
Delete(&ContentHistory{}); err != nil {
return
return nil, err
}

// Delete comments and attachments
if _, err = sess.In("issue_id", deleteCond).
Delete(&Comment{}); err != nil {
return
return nil, err
}

// Dependencies for issues in this repository
if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueDependency{}); err != nil {
return
return nil, err
}

// Delete dependencies for issues in other repositories
if _, err = sess.In("dependency_id", deleteCond).
Delete(&IssueDependency{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueUser{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&Reaction{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&IssueWatch{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&Stopwatch{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&TrackedTime{}); err != nil {
return
return nil, err
}

if _, err = sess.In("issue_id", deleteCond).
Delete(&project_model.ProjectIssue{}); err != nil {
return
return nil, err
}

if _, err = sess.In("dependent_issue_id", deleteCond).
Delete(&Comment{}); err != nil {
return
return nil, err
}

var attachments []*repo_model.Attachment
if err = sess.In("issue_id", deleteCond).
Find(&attachments); err != nil {
return
return nil, err
}

for j := range attachments {
Expand All @@ -770,11 +770,11 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []

if _, err = sess.In("issue_id", deleteCond).
Delete(&repo_model.Attachment{}); err != nil {
return
return nil, err
}

if _, err = db.DeleteByBean(ctx, &Issue{RepoID: repoID}); err != nil {
return
return nil, err
}

return attachmentPaths, err
Expand Down
18 changes: 9 additions & 9 deletions models/issues/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,18 @@ func init() {
// LoadCodeComments loads CodeComments
func (r *Review) LoadCodeComments(ctx context.Context) (err error) {
if r.CodeComments != nil {
return
return err
}
if err = r.loadIssue(ctx); err != nil {
return
return err
}
r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r, false)
return err
}

func (r *Review) loadIssue(ctx context.Context) (err error) {
if r.Issue != nil {
return
return err
}
r.Issue, err = GetIssueByID(ctx, r.IssueID)
return err
Expand All @@ -156,7 +156,7 @@ func (r *Review) loadIssue(ctx context.Context) (err error) {
// LoadReviewer loads reviewer
func (r *Review) LoadReviewer(ctx context.Context) (err error) {
if r.ReviewerID == 0 || r.Reviewer != nil {
return
return err
}
r.Reviewer, err = user_model.GetPossibleUserByID(ctx, r.ReviewerID)
return err
Expand Down Expand Up @@ -186,7 +186,7 @@ func LoadReviewers(ctx context.Context, reviews []*Review) (err error) {
// LoadReviewerTeam loads reviewer team
func (r *Review) LoadReviewerTeam(ctx context.Context) (err error) {
if r.ReviewerTeamID == 0 || r.ReviewerTeam != nil {
return
return nil
}

r.ReviewerTeam, err = organization.GetTeamByID(ctx, r.ReviewerTeamID)
Expand All @@ -196,16 +196,16 @@ func (r *Review) LoadReviewerTeam(ctx context.Context) (err error) {
// LoadAttributes loads all attributes except CodeComments
func (r *Review) LoadAttributes(ctx context.Context) (err error) {
if err = r.loadIssue(ctx); err != nil {
return
return err
}
if err = r.LoadCodeComments(ctx); err != nil {
return
return err
}
if err = r.LoadReviewer(ctx); err != nil {
return
return err
}
if err = r.LoadReviewerTeam(ctx); err != nil {
return
return err
}
return err
}
Expand Down
2 changes: 1 addition & 1 deletion models/issues/tracked_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func DeleteTime(t *TrackedTime) error {
func deleteTimes(ctx context.Context, opts FindTrackedTimesOptions) (removedTime int64, err error) {
removedTime, err = GetTrackedSeconds(ctx, opts)
if err != nil || removedTime == 0 {
return
return removedTime, err
}

_, err = opts.toSession(db.GetEngine(ctx)).Table("tracked_time").Cols("deleted").Update(&TrackedTime{Deleted: true})
Expand Down
Loading