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

openapi からprotoに変更 #10

Merged
merged 10 commits into from
Apr 10, 2024
Merged

Conversation

tosaken1116
Copy link
Collaborator

実装内容

全APIをprotoに変更

protoc pluginによるmsw handlerの生成

Copy link

@K-Kizuku K-Kizuku left a comment

Choose a reason for hiding this comment

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

ざっくりみた

Makefile Outdated
@@ -4,6 +4,7 @@ PROTO_FILES = $(shell find ./proto/schema -name "*.proto" -type f)
build:
rm -rf openapi
mkdir openapi
protoc -I. --plugin=./protoc-plugin/protoc-gen-ts-msw-handlers/protoc-gen-ts-msw-handlers --ts-msw-handlers_out=./msw ./proto/schema/**/*.proto
Copy link

Choose a reason for hiding this comment

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

各コマンドを.PHONYに書いた方がよき

}
export type GetUserWorksRequest = {
userId: string;
vislibility: string;
Copy link

Choose a reason for hiding this comment

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

visibility?

export type GetUserWorksRequest = {
userId: string;
vislibility: string;
newestUserId: string;
Copy link

Choose a reason for hiding this comment

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

newestよりlatestの方が自然ではある(けど多分ここですべきレビューではない)


option go_package = "./";
enum AssetType {
zip = 0;
Copy link

Choose a reason for hiding this comment

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

protoでは0に意味のある値を持たせないことが多いから0はUNKNOWNくらいにしておくのが一般的

}

message DeleteAssetResponse {
string status =1;
Copy link

Choose a reason for hiding this comment

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

Suggested change
string status =1;
string status = 1;

リンターがうまく働いてない?


message GetUserWorksRequest{
string user_id = 1;
string vislibility = 2;
Copy link

Choose a reason for hiding this comment

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

visibility?

Copy link

@K-Kizuku K-Kizuku left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@K-Kizuku K-Kizuku left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@K-Kizuku K-Kizuku left a comment

Choose a reason for hiding this comment

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

OK

@tosaken1116 tosaken1116 requested a review from K-Kizuku April 10, 2024 02:49
@tosaken1116 tosaken1116 merged commit 171f632 into main Apr 10, 2024
1 check passed
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.

2 participants