rbs icon indicating copy to clipboard operation
rbs copied to clipboard

use optional record keys for several API options

Open HoneyryderChuck opened this issue 1 year ago • 4 comments

HoneyryderChuck avatar Jun 18 '24 17:06 HoneyryderChuck

DRAFT:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests

ParadoxV5 avatar Jun 18 '24 18:06 ParadoxV5

@soutaro it's ready for review 🙏

HoneyryderChuck avatar Jun 26 '24 16:06 HoneyryderChuck

Thank you for your work! 🎉 I'm totally good for using optional records for return values.

However, using the records instead of Hash for options may result in reporting more type errors.

opts = {}
# Set up opts
CGI.new(opts)        # Type error because it's a Hash, not a record

What do you think about this? I'm thinking of if having additional rules for Hash -> record conversion in Steep would help.

soutaro avatar Jun 27 '24 00:06 soutaro

I'll run some tests locally to see if this impacts rbs runtime type check significantly. Nevertheless, I think there's great value in being precise on the options you accept in a hash as a function argument, particularly in examples such as ssl context, or URI build functions. That's definitely something steep could be a bit smarter about 👍

HoneyryderChuck avatar Jun 27 '24 06:06 HoneyryderChuck