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

增加 route_users 相關註解 #111

Conversation

y2468101216
Copy link
Collaborator

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

增加 route_users 相關的註解

@y2468101216
Copy link
Collaborator Author

大家可以看看這樣的註解是否恰當

internal/delivery/http/route_users_mock_test.go Outdated Show resolved Hide resolved
@@ -7,6 +7,8 @@ import (
"testing"
)

// ref - internal\delivery\http\route_users.go:30
Copy link
Member

Choose a reason for hiding this comment

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

這邊用 ref - 的用途是?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

註明他是測試哪邊的 code

Comment on lines 26 to 29
// ref - https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__information
// desc - 取得使用者資訊,包括上次上站位置等
// desc - get user information , include last visit
// route - /v1/users/{user_id}/information
// parameter - userID : 使用者 id
Copy link
Member

Choose a reason for hiding this comment

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

getUserInformation is a http handler function which will writes the information including last visit time of user with userID to w. request path should be /v1/users/{{user_id}}/information
Please see: https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__information

這樣應該會比較好?
雖然這邊是沒有 exported 的 method 也許也可以不用註解?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我覺得要,如果只註解有 export 的話 route 部分會看不太懂。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

desc 不要太長比較好,可以分拆就分拆,太長沒人想看,維護也比較方便

Copy link

@Yu-Jack Yu-Jack Mar 5, 2021

Choose a reason for hiding this comment

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

因為我看註解裡面寫的內容, 大多已經存在 swagger, 像是以下幾個內容都有

  1. route
  2. desc
  3. parameter

再加上 function 名稱其實已經明確叫做 getUserInformation
UserInformation 可以有很多定義, 最後一次上站時間也可以歸類在這
也就是說可以不用特地寫說 include last visit
所以我在想這邊, 改成這樣註解如何呢? (有兩個版本)

/*
    version1: 
    routes includes
        - getUserInformation: /v1/users/{user_id}/information
        - getUserFavorites: /v1/users/{user_id}/favorites
    
    version2: 
    routes includes
        - /v1/users/{user_id}/information
        - /v1/users/{user_id}/favorites
*/
func (delivery *Delivery) getUsers(w http.ResponseWriter, r *http.Request) {
	userID, item, err := parseUserPath(r.URL.Path)
	switch item {
	case "information":
		delivery.getUserInformation(w, r, userID)
    // 其餘省略
}

// ref - https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__information
func (delivery *Delivery) getUserInformation(w http.ResponseWriter, r *http.Request, userID string) {}

// ref - https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__favorites
func (delivery *Delivery) getUserFavorites(w http.ResponseWriter, r *http.Request, userID string) {}

我想說這樣寫可以讓別人快速理解這 route 有哪些 path
然後下面兩個 function, 則是直接把 swagger 連結放上去

只是我有個地方想請教一下, swagger 上面的內容都會是以中文為主還是英文呢?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

swagger 之前有私下討論過,因為現在是沒人維護的狀態,怕到時會不同步,所以是採取兩者並行。

Copy link
Member

Choose a reason for hiding this comment

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

格式上我不建議用 ref - 的這樣的格式

Copy link
Member

Choose a reason for hiding this comment

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

desc 不要太長比較好,可以分拆就分拆,太長沒人想看,維護也比較方便

我打的那串是希望取代掉整個註解,Golang 的註解通常會可以直接轉成 godoc, 那看到的東西會以敘述的方式呈現,因此不應該是把他另外拆成參數這樣下去寫。

// ref - https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__favorites
// desc - get user favorite list
// route - /v1/users/{user_id}/favorites
// parameter - userID : 使用者 id
Copy link

Choose a reason for hiding this comment

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

有考慮用 /* */ 去寫嗎? 感覺寫長一點的話, 會比較好讀?

/*

  ref - https://pttapp.cc/swagger/#/%E4%BD%BF%E7%94%A8%E8%80%85%E9%83%A8%E5%88%86/get_v1_users__user_id__favorites
  desc - get user favorite list
  route - /v1/users/{user_id}/favorites
  parameter - userID : 使用者 id

*/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

是可以

@y2468101216
Copy link
Collaborator Author

根據討論修改了 pr

@codecov-io
Copy link

codecov-io commented Mar 8, 2021

Codecov Report

Merging #111 (6f7abbd) into development_enable_lint (69983ea) will increase coverage by 2.15%.
The diff coverage is n/a.

Impacted file tree graph

@@                     Coverage Diff                     @@
##           development_enable_lint     #111      +/-   ##
===========================================================
+ Coverage                    29.16%   31.32%   +2.15%     
===========================================================
  Files                           20       20              
  Lines                          696      696              
===========================================================
+ Hits                           203      218      +15     
+ Misses                         463      445      -18     
- Partials                        30       33       +3     
Impacted Files Coverage Δ
internal/delivery/http/route_users.go 54.54% <ø> (+27.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69983ea...6f7abbd. Read the comment docs.

@@ -45,6 +49,9 @@ func TestGetUserInformation(t *testing.T) {
}
}

/*
TestParseUserPath is a test function which will test getUsers route mapping
*/
Copy link
Member

Choose a reason for hiding this comment

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

單行的話應該用 // 就行了?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我想說註解統一都用/**/

Copy link
Member

Choose a reason for hiding this comment

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

但是實際上也不太會通通都改成 /**/ 目前大部分的其實以 // 居多

改為英文

Co-authored-by: Pichu Chen <pichubaby@gmail.com>
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.

None yet

4 participants