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

feat: MVP1 development tasks. #54

Closed
wants to merge 17 commits into from
Closed

Conversation

IRONICBo
Copy link
Collaborator

@IRONICBo IRONICBo commented Jan 23, 2024

MVP1 Task: #38

@IRONICBo
Copy link
Collaborator Author

I will rebase commits later.

@IRONICBo IRONICBo changed the base branch from main to dev January 24, 2024 02:53
@IRONICBo IRONICBo requested a review from xushiwei January 24, 2024 02:53
.gitignore Outdated Show resolved Hide resolved
// Get User Info
var userClaim *casdoorsdk.Claims
token, err := ctx.Request.Cookie("token")
if err == nil {
Copy link
Member

Choose a reason for hiding this comment

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

Why is there no reverse judgment? like

if err != nil {}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I will update it.

"ID": id,
"Title": article.Title,
"Content": article.Content,
"Content": article.HtmlUrl,
"Tags": article.Tags,
Copy link
Member

Choose a reason for hiding this comment

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

gop format


get "/getArticle/:id", ctx => {
id := ctx.param("id")
article, _ := community.article(todo, id)
Copy link
Member

Choose a reason for hiding this comment

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

handle err

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Baihhh will fix it.

// Get User Info
var userClaim *casdoorsdk.Claims
token, err := ctx.Request.Cookie("token")
if err == nil {
Copy link
Member

Choose a reason for hiding this comment

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

handle err != nil scenario

}
}

get "/getArticle/:id", ctx => {
Copy link
Member

Choose a reason for hiding this comment

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

it's duplicate with get /p/:id ? Also, it's better to use unified style for router,

var htmlId string
sqlStr := "select id,title,user_id,cover,tags,content,html_id,ctime,mtime from article where id=?"
err = p.db.QueryRow(sqlStr, id).Scan(&article.ID, &article.Title, &article.UId, &article.Cover, &article.Tags, &article.Content, &htmlId, &article.Ctime, &article.Mtime)
if err == sql.ErrNoRows {
Copy link
Member

Choose a reason for hiding this comment

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

when err == nil, current logic still need two check operations which seems wasteful.

suggest:

if err != nil {
    if err == ErrNotExist {
        return article, ErrNotExist 
     }
     log.Error("unexpected err", err)
      return article, err
}

} else if err != nil {
return article, err
}
// add author info
Copy link
Member

Choose a reason for hiding this comment

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

recommend to remove useless code

var htmlId string
sqlStr := "select trans_html_id from article where id=?"
err = p.db.QueryRow(sqlStr, id).Scan(&htmlId)
if err == sql.ErrNoRows {
Copy link
Member

Choose a reason for hiding this comment

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

same with above.

}
//line cmd/gopcomm/community_yap.gox:11
//line cmd/gopcomm/community_yap.gox:23
func (this *community) MainEntry() {
Copy link

Choose a reason for hiding this comment

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

[staticcheck] receiver name should be a reflection of its identity; don't use generic names such as "this" or "self" (ST1006)

}

type Account struct {
casdoorConfig *CasdoorConfig
Copy link

Choose a reason for hiding this comment

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

[staticcheck] field casdoorConfig is unused (U1000)

type Account struct {
casdoorConfig *CasdoorConfig

zlog *zap.SugaredLogger
Copy link

Choose a reason for hiding this comment

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

[staticcheck] field zlog is unused (U1000)

func (p *Community) PutArticle(ctx context.Context, uid string, article *Article) (id string, err error) {
// SaveHtml upload origin html(string) to media for html id and save id to database
func (p *Community) SaveHtml(ctx context.Context, uid, htmlStr, mdData, id string) (articleId string, err error) {
htmlId, err := p.SaveMedia(ctx, uid, []byte(htmlStr))
Copy link

Choose a reason for hiding this comment

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

[staticcheck] this value of err is never used (SA4006)

if err != nil {
return "", err
}
articleId, err := res.LastInsertId()
Copy link

Choose a reason for hiding this comment

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

[staticcheck] this value of err is never used (SA4006)

if err != nil {
return "", err
}
idInt, err := res.LastInsertId()
Copy link

Choose a reason for hiding this comment

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

[staticcheck] this value of err is never used (SA4006)

var htmlIds []string
sqlStr := "select html_id from article where user_id=?"
rows, err := p.db.Query(sqlStr, uid)
defer rows.Close()
Copy link

Choose a reason for hiding this comment

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

[staticcheck] should check returned error before deferring rows.Close() (SA5001)


bucket := c.bucket
r, err := bucket.NewReader(context.Background(), fileKey, nil)
defer r.Close()
Copy link

Choose a reason for hiding this comment

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

[staticcheck] should check returned error before deferring r.Close() (SA5001)

mockAccessKey = os.Getenv("QINIU_ACCESS_KEY")
mockSecretKey = os.Getenv("QINIU_SECRET_KEY")
mockBucket = os.Getenv("QINIU_TEST_BUCKET")
mockPipeline = os.Getenv("QINIU_TEST_PIPELINE")
Copy link

Choose a reason for hiding this comment

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

[staticcheck] var mockPipeline is unused (U1000)

Copy link

qiniu-x bot commented Jan 31, 2024

[REBASE SUGGESTION] Hi @IRONICBo, your PR has commit messages like:

Merge branch 'dev' into docs/function-modules

Merge branch 'merge' into feat/mvp-20240119

Which seems insignificant, recommend to use git rebase <upstream>/<branch> to reorganize your PR.

If you have any questions about this comment, feel free to raise an issue here:

@IRONICBo
Copy link
Collaborator Author

This PR have already moved to dev.

@IRONICBo IRONICBo closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants