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

add host config #57

Merged
merged 9 commits into from Sep 5, 2023
Merged

add host config #57

merged 9 commits into from Sep 5, 2023

Conversation

ikepu-tp
Copy link
Contributor

@ikepu-tp ikepu-tp commented Sep 2, 2023

What

  • You can configure host such as localhost, 0.0.0.0 and so on with config file.

How

  • Set host on qiita.config.json.

Why

  • people cannot set host.

Refs

**Japanese is better. (I'm Japanese.)

@ohakutsu
Copy link
Member

ohakutsu commented Sep 4, 2023

@ikepu-tp
Pull Requestありがとうございます!
内容確認させていただきます!

@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 4, 2023

@ohakutsu
ありがとうございます。テストが失敗した内容を確認し,修正いたします。
何卒宜しくお願い致します。

@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 4, 2023

@ohakutsu
修正しましたので再度ご確認をお願いいたします。

@ohakutsu
Copy link
Member

ohakutsu commented Sep 5, 2023

@ikepu-tp
修正ありがとうございます!確認させていただきます!

Dockerfile, docker-compose.ymlはQiita CLI側で提供せず、ユーザー側で自由に環境をつくってもらうようにできればと思っていたのですが、いかがでしょうか?

@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 5, 2023

@ohakutsu

Qiita CLIをDockerで構築する方法について,Qiitaに投稿してみたところ,自分の記事の中では一番閲覧数が多く,Dockerで構築することは一定の需要があるように思いました。そこでQiita CLIのinitDockerfiledocker-compose.ymlが含まれると手軽に始められて良いかなと思い,提案させて頂きました。(人生初PRでご無礼ありましたら申し訳ないです)

ただ,Dockerfiledocker-compose.ymlをユーザー側が作るとしても現Qiita CLIではホスト部の設定ができず,これらのファイルを作るだけでは利用できません。今,私はnode_modulesのファイルを直接編集し,無理やり使っている状態です。ホスト部の設定ができるようにすることは必要ではないかと思いますが,いかがでしょうか。(cf. e4d8904, 790e4c3, 79283e6 )

@ohakutsu
Copy link
Member

ohakutsu commented Sep 5, 2023

@ikepu-tp
ありがとうございます!

コメントいただいた通り、ホスト部の指定ができるようにはしたいと考えておりました。
ただ、Dockerfile, docker-compose.yml自体はinitコマンドでは生成せず、自由につくっていただく様にできればと考えています。

もし、ホスト部の指定のみで課題解決ができるようであれば、こちらのPull Requestをホスト部の指定のみにしていただけますでしょうか?

@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 5, 2023

@ohakutsu
ありがとうございます。Docker関連のコードをすべて削除したのでご確認お願い致します。

@ohakutsu
Copy link
Member

ohakutsu commented Sep 5, 2023

@ikepu-tp
ありがとうございます!確認します!

Pull Requestの説明の更新もお願いします 🙇
#57 (comment)

@ikepu-tp ikepu-tp changed the title add docker and address config add address config Sep 5, 2023
Copy link
Member

@ohakutsu ohakutsu left a comment

Choose a reason for hiding this comment

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

レビューさせていただきました!

src/lib/config.ts Outdated Show resolved Hide resolved
src/server/app.ts Outdated Show resolved Hide resolved
src/lib/config.ts Outdated Show resolved Hide resolved
@ikepu-tp ikepu-tp changed the title add address config add host config Sep 5, 2023
@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 5, 2023

@ohakutsu

レビューありがとうございました。プログラムの全体を把握できていなかったみたいで申し訳ないです。
ご指摘いただいた箇所を修正しました。
再度ご確認をお願いいたします

Copy link
Member

@ohakutsu ohakutsu left a comment

Choose a reason for hiding this comment

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

LGTM!
修正いただきありがとうございます!

@ohakutsu
Copy link
Member

ohakutsu commented Sep 5, 2023

Pull Requestありがとうございました!
mergeさせていただきます!

@ohakutsu ohakutsu merged commit e07c04e into increments:main Sep 5, 2023
5 checks passed
@ikepu-tp
Copy link
Contributor Author

ikepu-tp commented Sep 5, 2023

@ohakutsu

マージありがとうございます!

@ohakutsu ohakutsu mentioned this pull request Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants