Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for generating RBIs for Rails engines #1220

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

andyw8
Copy link
Contributor

@andyw8 andyw8 commented Oct 7, 2022

(Paired with @vinistock)

Motivation

Closes #1182

Implementation

An --app_root option was added to the tapioca dsl command. This can be used to specify the path to the dummy app for an engine.

There were several places in the code which assumed the app is always at the root of the project. These were updated to take the app_root option into consideration.

Tests

The test approach is styled after the existing RBI generation tests. We create files for the app as normal, but then move the necessary files into the test/dummy path before running the dsl commmand.

@andyw8 andyw8 changed the title Add support for Rails engines Add option for generating RBIs for Rails engines Oct 7, 2022
@andyw8 andyw8 force-pushed the andyw8/vinistock/add-support-for-rails-engines branch 2 times, most recently from a665be6 to c726440 Compare October 7, 2022 10:04
@andyw8 andyw8 marked this pull request as ready for review October 7, 2022 13:29
@andyw8 andyw8 requested a review from a team as a code owner October 7, 2022 13:29
@andyw8 andyw8 force-pushed the andyw8/vinistock/add-support-for-rails-engines branch from c726440 to 3ce0dba Compare October 8, 2022 11:15
begin
FileUtils.mkdir_p(engine_path)
FileUtils.mv(@project.path + "/config", engine_path)
FileUtils.mv(@project.path + "/lib", engine_path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer writing the files to /test/dummy directly on line 347 but not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, unfortunately the top-level before(:all) sets the project path so we would need to restructure the tests first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused. I was only referring to the lib files. More specifically I was thinking of changing the 2 writes to:
@project.write("test/dummy/lib/post.rb", <<~RB) & @project.write("test/dummy/lib/comment.rb", <<~RB)

and removing the line FileUtils.mv(@project.path + "/lib", engine_path).

We don't have to do it, I'm just trying to understand the project path blocker 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand now. I was thinking of config/environment.rb which gets created in the before(:all). I've opened #1225 to address this.

@andyw8 andyw8 merged commit b70c59e into main Oct 12, 2022
@andyw8 andyw8 deleted the andyw8/vinistock/add-support-for-rails-engines branch October 12, 2022 12:03
@shopify-shipit shopify-shipit bot temporarily deployed to production November 10, 2022 17:34 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow generating RBIs in Rails engines
3 participants