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

第2章2回の修正 #92

Merged
merged 4 commits into from
Apr 12, 2024
Merged

第2章2回の修正 #92

merged 4 commits into from
Apr 12, 2024

Conversation

nokhnaton
Copy link
Contributor

実演中に見つけた問題点

@Takeno-hito
Copy link
Member

追加で、このあたりを直してほしいです。

  • proxy のところ:プロキシじゃなくてCORSの設定をしたくないですか?わざわざプロキシを使う理由はないと思います
  • whoami エンドポイントじゃなくて me の方が一般的では?
  • プロキシする という言い回しはあんまり正式な用法じゃないと思います、少しひっかかります
  • 「CORS を回避する」という言い方はあんまり好きではないです。CORS を正しく設定する、とかにするべきでは?

@hijiki51
Copy link
Member

今回のような手元で立てる場合はProxyのほうが一般的なのでproxyでいいと思います(CORSになるのは手元環境による副作用であって本質ではないため)

@hijiki51
Copy link
Member

プロキシする => プロキシを挟む とかかな?

@nokhnaton
Copy link
Contributor Author

/whoamiは別に/meに書き換えなくてもいいかなって思ったので直してないです

Copy link
Member

@Takeno-hito Takeno-hito left a comment

Choose a reason for hiding this comment

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

申し訳ないですがはっきり言って /whoami はナシだと思います。URI はリソースを表現することが望ましいと言われています。
たとえ /whoami にしたままにするとしても「traQ やその他 traP のアプリケーションでの書き方に習って」ないのでこの文面の修正は必要です。

API 設計に関してはこのあたりの記事が参考になると思います。
https://note.com/yamarkz/n/n41e9ac83c896

docs/chapter2/section2/1_proxy.md Outdated Show resolved Hide resolved
@nokhnaton
Copy link
Contributor Author

URI はリソースを表現することが望ましいと言われています。

参考として挙げられたURLに具体的に書かれてなかったので、私の解釈も含みますが、whoamiでも十分シンプルで直感的だと思います。
meに変えるでも別に良いとは思いますが、第一章も書き換えなければいけないので、このプルリクではやりたくないです。また、変えるならその告知はしたいです。

「traQ やその他 traP のアプリケーションでの書き方に習って」ない

「traQ やその他 traP のアプリケーションでの書き方に習って」いるのは、エンドポイントを叩いてログインしているか確認するところで、実際「traQ でも一番始めに whoami と同じ働きをするエンドポイントを叩き自分の情報を取得しています。」とあります

Co-authored-by: Take <18237819+Takeno-hito@users.noreply.github.com>
@Takeno-hito Takeno-hito dismissed their stale review September 10, 2023 00:21

Dismiss the review

@Takeno-hito Takeno-hito merged commit db2063f into main Apr 12, 2024
@Takeno-hito Takeno-hito deleted the 2-2-fetch branch April 12, 2024 01:30
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.

3 participants