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.

Sign up for GitHub

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

Jump to bottom

CreateServerLocationsの実装 #83

Merged
merged 19 commits into from
Jul 22, 2022
Merged

CreateServerLocationsの実装 #83

merged 19 commits into from
Jul 22, 2022

Conversation

Yamada-Ika
Copy link
Collaborator

@Yamada-Ika Yamada-Ika commented Jul 21, 2022 •
edited
Loading

できていること

  • 単純なconfigファイルを想定し、期待通りのServerLocationを生成

できていないこと

  • autoindexがoffになっているか、そもそも設定されていないかが判別できない
  • stringで持っている変数も同じかも

詳細はテストケースに書いています

@Yamada-Ika Yamada-Ika linked an issue Jul 21, 2022 that may be closed by this pull request
CreateServerLocationsを実装 #74
Closed
@Yamada-Ika
Copy link
Collaborator Author

まだ動作しないが、大まかな処理を実装した
if文が羅列していて醜いのどうにかしたい

@Yamada-Ika
Copy link
Collaborator Author

Yamada-Ika commented Jul 21, 2022 •
edited
Loading

boolで持っているが故に、configファイルに値が設定されているかどうかがわからない
例 ) autoindex

@Yamada-Ika Yamada-Ika force-pushed the feat/74_createserverlocations branch from d43d7b8 to 40000cf Compare July 22, 2022 02:49
@Yamada-Ika Yamada-Ika marked this pull request as ready for review July 22, 2022 03:55
hayashi-ay
hayashi-ay approved these changes Jul 22, 2022
View reviewed changes
Copy link
Collaborator

@hayashi-ay hayashi-ay left a comment

Choose a reason for hiding this comment

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

良さそうです。細かいのコメントを2点しています。

srcs/Webserv/Webserv.cpp Outdated Show resolved Hide resolved
srcs/Webserv/ServerLocationFacade.cpp Outdated Show resolved Hide resolved
@katataku katataku mentioned this pull request Jul 22, 2022
Merged
@hayashi-ay hayashi-ay merged commit 9dec22c into main Jul 22, 2022
@hayashi-ay hayashi-ay deleted the feat/74_createserverlocations branch July 22, 2022 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hayashi-ay hayashi-ay hayashi-ay approved these changes

@katataku katataku katataku approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CreateServerLocationsを実装
3 participants
@Yamada-Ika @katataku @hayashi-ay