gem_rbs_collection

gem_rbs_collection copied to clipboard

This PR partially addresses #210 (the updating action, not the Rack 3 concern) by reviewing and updating some of Rack’s commonly-used classes:

Builder
Config
Lobster
MediaType
MockRequest
MockResponse
Request
Response

• Rack::Response was highlighted in #210’s blockquote.
• I am certain that (the signatures of) these classes were not modified from Rack 2.2.2 to 2.2.4 (#215).
• The following items require a bit of follow-up:
  • uses a duck type specified in Rack’s SPEC which I have not written eagerly
    • Return of Request::Helpers#body
    • Return of Request::Helpers#logger
    • Argument errors of MockResponse#initialize
  • requires diving deeper into Rack’s mechanics or specifications
    • Return of Request::Helpers#session_options
    • Return of Request::Helpers#parse_multipart

---

I may have an interest in additionally volunteering for some middleware (but not all of the) classes after this PR. The intention is to ship the above classes first to cover most of Rack’s use cases (hopefully – I don’t wanna do the entire thing on my own 😣).

• Check (link):

Skipping: cannot find test script at `gems/rack/2.2/_scripts/test`...

• runs bundle exec steep check on local machine: No type error detected. 🫖

• Check (link):

Skipping: cannot find test script at `gems/rack/2.2/_scripts/test`...

• runs bundle exec steep check on local machine: No type error detected. 🫖

Ah, the test on CI is skipped because _scripts/test file doesn't exist. I prefer enabling CI for rack gem, so could you add this file? You can find examples from other gems, and the boilerplate generator.

I may have an interest in additionally volunteering for some middleware (but not all of the) classes after this PR. The intention is to ship the above classes first to cover most of Rack’s use cases (hopefully – I don’t wanna do the entire thing on my own 😣).

Thanks for your volunteering 🙌 I think it is not a problem that you do not write RBS for entire of the gem. It is better if you can make a task list for classes/modules that needs updates. It will be helpful for other contributors.

It is better if you can make a task list for classes/modules that needs updates. It will be helpful for other contributors.

Insightful suggestion! I compiled a list at https://github.com/ruby/gem_rbs_collection/issues/210#issuecomment-1278055273.

P.S. I also looked at [Rack Top-Level] and RegexpExtensions during this PR's commits and found no need for changes.

Ah, the test on CI is skipped because _scripts/test file doesn't exist. I prefer enabling CI for rack gem, so could you add this file? You can find examples from other gems, and the boilerplate generator.

Yep, I looked into it after my comment and learned that the CI check relies on _scripts/test. I tinkered a li’l bit but am not confident about where to list the newly referenced dependencies as included in 20a2571.

P.S. Do I list them in manifest.yaml as well as Steepfile?

Insightful suggestion! I compiled a list at https://github.com/ruby/gem_rbs_collection/issues/210#issuecomment-1278055273.

Great! Thank you 👍 I'll share this list with colleagues who're interested in contributing to RBS.

Yep, I looked into it after my comment and learned that the CI check relies on _scripts/test. I tinkered a li’l bit but am not confident about where to list the newly referenced dependencies as included in https://github.com/ruby/gem_rbs_collection/commit/20a25713cb093fbf3cf960e0f9b18f5a6053436e.

P.S. Do I list them in manifest.yaml as well as Steepfile?

Ah, good question. Currently, we need to write the dependencies to the three places, which are Steepfile, _scripts/test, and manifest.yaml. It is really inconvenient, so I'm trying to resolve this problem. Sorry for the inconvenient 🙏

---

By the way, only dependencies that are not in the gemspec should be written in the manifest.yaml. rack has two dependencies, cgi and uri. Both dependencies aren't listed in the gemspec, so we need to write both gems to manifest.yml.

Inconvenient indeed, 7262031 fails because the CI tested the Rack RBS using test scripts for Active Record and Action Pack simply because those gems are out of date vs. main.