pocke / rbs_rails

Apache License 2.0
282 stars 33 forks source link

Create signature_root_dir before generating path_helpers #256

Closed tk0miya closed 1 year ago

tk0miya commented 1 year ago

Now, generate_rbs_for_path_helpers task does not check the signature_root_dir present. So it would be crashed if sig/rbs_rails/ directory is not found.

pocke commented 1 year ago

setup_signature_root_dir! method creates signature_root_dir if it does not exist. So I guess this change is unnecessary.

Could you explain the error if you got some error on this rake task?

tk0miya commented 1 year ago

I'm using rbs.rake based on https://github.com/pocke/rubykaigi-2023-lets-write-rbs/blob/master/rbs.rake. And I got the following error in my GitHub Actions:

rails aborted!
Errno::ENOENT: No such file or directory @ rb_sysopen - /home/runner/work/xxx/xxx/sig/rbs_rails/path_helpers.rbs
/home/runner/work/xxx/xxx/vendor/bundle/ruby/3.1.0/gems/rbs_rails-0.12.0/lib/rbs_rails/rake_task.rb:63:in `write'
/home/runner/work/xxx/xxx/vendor/bundle/ruby/3.1.0/gems/rbs_rails-0.12.0/lib/rbs_rails/rake_task.rb:63:in `write'
/home/runner/work/xxx/xxx/vendor/bundle/ruby/3.1.0/gems/rbs_rails-0.12.0/lib/rbs_rails/rake_task.rb:63:in `block in def_generate_rbs_for_path_helpers'
(snip)
Tasks: TOP => rbs:setup => rbs_rails:all => rbs_rails:generate_rbs_for_path_helpers
(See full trace by running task with --trace)
Error: Process completed with exit code 1.

I suppose this error happened with the following steps:

  1. On loading rbs.rake, sig/rbs_rails directory was created via RbsRails::RakeTask.new.
  2. At the first step of rbs:setup task, sig/rbs_rails directory was removed via rbs:clean task.
  3. On the subsequent step; rbs_rails:generate_rbs_for_path_helpers task, ENOENT error happened.

This is why I need to create the directory just before generating path_helpers.

pocke commented 1 year ago

Thanks for your explanation 👍 I understand the problem.

But I'd like to fix this problem with another approach because I think creating the directory in initialize is a bad idea. ref: #261

Thanks for your PR but I'll close this PR 🙇

tk0miya commented 1 year ago

Thank you for your work. #261 seems good to me :-)