Skip to content

Conversation

@matsumotory
Copy link
Contributor

h2o_mruby support H2O class of mruby. For now, I implemented H2O.max_headers method.

H2O.max_headers #=> 100

@kazuho
Copy link
Member

kazuho commented Jul 5, 2015

Thank you for the PR. The code looks good to me.

OTOH H2O_MAX_HEADERS is a limitation only applied to the HTTP parsers (you can add / emit any number of headers from mruby), and therefore I do not see any particular reason for exposing the value.

How about adding other features and then merging to master?

EDIT. FYI we use clang-format for indenting the source files. Applying clang-format -i to the source files is appreciated (or you can run misc/clang-format-all.sh to indent all the files at once).

@matsumotory
Copy link
Contributor Author

Thank you for your review.

I do not see any particular reason for exposing the value.
How about adding other features and then merging to master?

I wanted to implement a simple method of H2O class for now, but I feel fairly sure that max_headers method is impracticable. So, I try to add other practicable feature like error logging or header control.

Applying clang-format -i to the source files is appreciated (or you can run misc/clang-format-all.sh to indent all the files at once).

Thank you for the notice. I see!

@matsumotory
Copy link
Contributor Author

@kazuho sorry for my late response. I implemented error log method and applied clang-format, but I'm not sure what to do about test of error log method. What do you think about this test?

@kazuho
Copy link
Member

kazuho commented Jul 11, 2015

@matsumoto-r Thank you for the updates. The code looks good to me.

I'm not sure what to do about test of error log method. What do you think about this test?

I would not request every method to have a test. In this case, I think we can go without having one.

@matsumotory
Copy link
Contributor Author

@kazuho Thank you for your thoght. So, I had just finished the scope of this PR which include H2O basic class for now. Please check it.

kazuho added a commit that referenced this pull request Jul 14, 2015
[h2o_mruby] Add H2O class and simple method
@kazuho kazuho merged commit 83d9825 into h2o:master Jul 14, 2015
@kazuho
Copy link
Member

kazuho commented Jul 14, 2015

Sorry for being late. LGTM. Merged to master.

@matsumotory
Copy link
Contributor Author

Thanks!!

@kazuho kazuho added the mruby label Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants