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

fix message #375

Closed

Conversation

k70suK3-k06a7ash1
Copy link
Contributor

Issue #, if available:
#338

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

bin.sh Show resolved Hide resolved
bin.sh Show resolved Hide resolved
bin.sh Outdated Show resolved Hide resolved
@k70suK3-k06a7ash1 k70suK3-k06a7ash1 marked this pull request as ready for review June 17, 2024 01:54
@k70suK3-k06a7ash1
Copy link
Contributor Author

@statefb
こちら修正しました!

docs/README_ja.md Outdated Show resolved Hide resolved
@k70suK3-k06a7ash1
Copy link
Contributor Author

@statefb
修正しました!
多分大丈夫かと・・・

README.mdの方に

- After about 30 minutes, you will get the following output, which you can access from your browser

がありましたが、おそらく文脈的につながらないように見えたのでen,jaともに該当箇所を削除してますmm

@k70suK3-k06a7ash1
Copy link
Contributor Author

でも、bash.shの実行後に表示される部分を以下〇〇、として記載した方が良いような気もしてきました・・・

パラメータを指定したコマンド例

の部分の出力結果に近しいものが出力されるかなと思いますので、如何しましょう?

@@ -54,7 +54,8 @@ chmod +x bin.sh
./bin.sh
```

- 新規ユーザーまたは v1 ユーザーかどうかを聞かれます。その場合は `y` を入力してください。
- v0.x ユーザー(以前のバージョンをすでに利用しているか)を聞かれます。そうではない場合は `n` を入力してください。
- もし v0.x ユーザーの場合は[マイグレーションガイド](./migration/V0_TO_V1.md)に従ってください。
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 「もしv0.x〜」は階層一段下げてください!
  • n を入力しデプロイします。の方が何が起きるか明確で親切かと思いました
  • Yのケースも記載してみましたが、いかがでしょう?

  • v0.x ユーザー(以前のバージョンをすでに利用しているか)を聞かれます。そうではない場合は n を入力しデプロイします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0857348

こちらで対応しました!

@@ -72,7 +73,7 @@ chmod +x bin.sh
./bin.sh --disable-self-register --ipv4-ranges "192.0.2.0/25,192.0.2.128/25" --ipv6-ranges "2001:db8:1:2::/64,2001:db8:1:3::/64" --allowed-signup-email-domains "example.com,anotherexample.com" --region "ap-northeast-1"
```

- 30 分ほど経過後、下記の出力が得られるのでブラウザからアクセスします
- 35 分ほど経過後、下記の出力が得られるのでブラウザからアクセスします
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 記載場所が違いますが、デプロイ時に以下のパラメータを指定することで、セキュリティとカスタマイズを強化でき「ます」に修正いただけますか?(本PRに関係ないのですが修正いただけると助かります)

Copy link
Contributor

@statefb statefb Jun 18, 2024

Choose a reason for hiding this comment

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

の部分の出力結果に近しいものが出力されるかなと思いますので、如何しましょう?

ご提案いただいた通り、確かに、「35 分ほど経過後、〜./bin を実行する際に disable-self-register を使用してセルフサインアップを無効にしてください。」をオプションの前に記載した方が読みやすいですね!

Importantの内容も下記のように修正した方が、オプションのセクションに繋がりやすいかと思いますが、いかがでしょうか?

Important
オプションのパラメータを設定しない場合、このデプロイ方法では URL を知っている誰でもサインアップできてしまいます。本番環境で使用する場合は、セキュリティリスクを軽減するために、IP アドレスの制限を追加し、セルフサインアップを無効にすることを強くお勧めします(allowed-signup-email-domains を定義して、会社のドメインからのメールアドレスのみがサインアップできるようにすることで、ユーザーを制限できます)。IP アドレスの制限には ipv4-ranges と ipv6-ranges の両方を使用し、./bin を実行する際に disable-self-register を使用してセルフサインアップを無効にしてください。詳しくは以下のオプションのパラメータをご確認ください。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

文末修正:「ます」に変更

e67065f
こちらで対応しました!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

セルフサインアップ無効の文言とImportantのセクションを追加しました!
e8e7bd9

@k70suK3-k06a7ash1
Copy link
Contributor Author

デプロイ時に以下のパラメータを指定することで、セキュリティとカスタマイズを強化でき「ます」に修正いただけますか?

こちら、承知です!

@statefb
Copy link
Contributor

statefb commented Jun 25, 2024

@k70suK3-k06a7ash1 ありがとうございます!改めて見直していて思ったのですが、

  • nでデプロイが挙動として分かりにくい
  • 重要な意思決定選択である一方、既にリリース済みのため論理反転変更することにリスクがある

以上を踏まえると、元のy/Nに戻すべきかなーと考えています。元々のorがわかりにくい件については、下記のようにすれば回避可能かなと思っています。

Were you using the old versions v0.x or prior, or are you a new user starting with v1.x or later? (y/N):

もちろん、これまで手を加えていただいたREADMEの整理はそのままマージさせていただければありがたいです!
(色々と手を加えていただいたにも関わらず申し訳ないですmm)

@k70suK3-k06a7ash1
Copy link
Contributor Author

@statefb

重要な意思決定選択である一方、既にリリース済みのため論理反転変更することにリスクがある

この点、確かにおっしゃる通りですね・・・
元に戻す方針、賛成です!

@k70suK3-k06a7ash1
Copy link
Contributor Author

@statefb
多分、このPRを修正するより、ゼロから作成して対応した方が影響範囲狭い気がしたので、このままCloseいただいても全然構いませんので!
(お気遣いいただいてたらと思い、念の為・・・)

@statefb
Copy link
Contributor

statefb commented Jun 25, 2024

@k70suK3-k06a7ash1 承知です!確かに新規で作り直した方が早いかもしれないですね。ありがとうございます!

@statefb
Copy link
Contributor

statefb commented Jun 25, 2024

duplicated #402

@statefb statefb closed this Jun 25, 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.

2 participants