Rack::Builder #use, #run and #map all return nil.
#2355
Conversation
jeremyevans
left a comment
There was a problem hiding this comment.
I agree we should be explicit in the return value. My preference is to have the methods return nil, so we can potentially use a different return value in the future. If we return self, we are basically stuck with that going forward, even if later we find it would be better to return something else. I don't think the minor benefit of supporting method chaining is worth potentially limiting ourselves in the future. If we choose to return self and not nil, we should add specs for the behavior we want to allow.
|
Thanks, I'm fine with either, so I'll change it to Just for future reference, apart from |
8678fdb to
dbf39dd
Compare
Rack::Builder #use, #run and #map all return self.Rack::Builder #use, #run and #map all return nil.
I'm not sure. I don't have any ideas yet. But since there's no burning need to support method chaining for this, I'd rather leave the option open. |
Rack::Builder#run returns nil, see rack/rack#2355
Rack::Builder#run returns nil, see rack/rack#2355
See #2346 for more context.
Alternatively, it could return
nil, but I think returningselfis slightly more useful. However, it does lock in the return value (nilmight give us more flexibility in the future).In principle with this change, one could write:
I'm open to either idea. WDYT.