Fix README inaccuracies
I used the README as a reference the other day and noticed it needed to be fixed. As I looked further, basically every section needed to be fixed or improved. It took a while to go through everything but I'm glad I did and we can have an accurate README. I went beyond just fixing the bare minimum and improved things as I went along. I found some bugs / potential improvements too. There's a good number of TODO's below, but I'd love feedback (and also check my work! I'm sure I made mistakes).
Summary
The biggest, nearly ubiquitous, fix was removing injected configuration objects, since we don't do that anymore.
The other major change was to make all code examples self-contained. You should be able to copy and paste them directly into IRB and they should "Just Work". This means adding some extra irrelevant boilerplate (e.g. ArticleRepo and Article), but I think we can trust readers to ignore those.
Minor changes:
- Changed ArticleRepository to ArticleRepo, to match our naming convention in hanami-db. We don't interact with the db layer at all here, of course, but it's good to be consistent to minimize confusion.
- We had Rack array responses, e.g.
# => [200, {}, ["body"]in many places but that's inaccurate. We get backHanami::Action::Responsenow. So users need to call#to_a(alias for#finish) to get the Rack-compatible array response. I started doing this throughout but I realized it could be broken up into several different lines for clarity, to only show the relevant info (via#status,#headers,#body, etc). This is more clear but also the headers used to be{}by default, and now they have Content-Type and Content-Length headers so it was very noisy (or had to be abbreviated as...which isn't valid Ruby here). - Standardized on
pinstead ofputs. I was using both, depending on the situation, but it seemed distracting. The main issue is thatputsalways returnsnil, so#=> 123was describing the printed value instead of the return value. I'm open to changing this, I just wanted to be consistent.
TODO
-
Structure
- [ ] Restructure order to surface most relevant info first, rewrite accordingly
- [ ] Add table-of-contents
- [ ] Rewrite intro and section copy. I don't think hanami-controller can be considered a "micro library" anymore
-
Rack
- [ ] Add note about supporting Rack 2 and Rack 3 (and rewindable input?)
- [ ] Add relevant info about how Rack works and how this library builds on top of it
- [ ] Add streaming request example
-
Formats
- [ ] Add note about surprising behavior with
config.formats.add - [ ] Add note about default format, including charsets
- [ ] Add note about default Content-Type and Content-Length headers
- [ ] Add note about surprising behavior with
-
Other:
- [ ] Add note about using symbols for statuses, i.e. :unauthorized work as well as 401
- [x] Change requires to
require "hanami/action"? Slightly confusing but we can add a note about it. - [ ] Add note about how sessions work in rack
- [ ] Do a final manual check of every example and section
Notes:
- I removed the streaming example because it didn't work and AFAICT we don't actually support streaming responses right now
- I removed a section about inheritance and error handling. There's nothing surprising about it, we can assume readers know how inheritance works in Ruby
Bugs / Potential improvements:
I plan to open issues for all of these. I'll replace them with links to the Issues when I do that.
- [ ]
request.cookiesonly allow string key access (and it's a plain::Hash).request.sessionautomatically coerces keys to symbols, and can only be accessed via symbols only. We should rationalize these somehow, it's confusing they're so different. - [ ] Fix surprising behavior with
config.formats.add? - [ ] Using
expires 600, :public, max_age: 123adds two max-age values, e.g. Expires header ispublic, max-age=123, max-age=600. Maybe we can deprecate having both? I think I'd prefer max_age as a kwarg only and removing the first arg option. - [ ] Reading cookies works fine without
include Hanami::Action::Cookies(so I removed it from that example). However, setting cookies does need thatinclude, else you get a'Hash#fetch': key not found: "Set-Cookie" (KeyError) - [ ] Setting
max_ageon a cookie automatically computes anExpiresas well. Using themax_agesetting from config does not set theExpires(you can see this in theSetCookiesexample) - [ ] Add in Streaming Body support, as defined in the Rack 3 spec: https://github.com/rack/rack/blob/main/SPEC.rdoc#streaming-body-. Ensure we support Enumerable bodies as specified too.
- [ ] Open issue outlining a proposed backward-compatible transition to hanami-action as the gem name