Skip to content

Loaded the directory under tasks.#747

Merged
rafaelfranca merged 2 commits into
rails:masterfrom
Mitsuru53:Mitsuru53/load-dir-under-tasks
Jun 1, 2021
Merged

Loaded the directory under tasks.#747
rafaelfranca merged 2 commits into
rails:masterfrom
Mitsuru53:Mitsuru53/load-dir-under-tasks

Conversation

@Mitsuru53

Copy link
Copy Markdown

What I did

This PR is to fix the directory that thor goes to look for

When an arbitrary directory is created under the tasks directory, the thor files in that directory will also be read.

Comment thread lib/thor/util.rb Outdated
def globs_for(path)
path = escape_globs(path)
["#{path}/Thorfile", "#{path}/*.thor", "#{path}/tasks/*.thor", "#{path}/lib/tasks/*.thor"]
["#{path}/Thorfile", "#{path}/*.thor", "#{path}/tasks/*.thor", "#{path}/lib/tasks/*.thor", "#{path}/lib/tasks/**/*.thor"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this also work?

Suggested change
["#{path}/Thorfile", "#{path}/*.thor", "#{path}/tasks/*.thor", "#{path}/lib/tasks/*.thor", "#{path}/lib/tasks/**/*.thor"]
["#{path}/Thorfile", "#{path}/*.thor", "#{path}/tasks/*.thor", "#{path}/lib/tasks/**/*.thor"]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rafaelfranca
Thank you for the review!
Yes, that works too! My code was redundant, I fix it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@rafaelfranca
Fixed.
I have confirmed that the specs all passed.
Please review again 😌

@Mitsuru53 Mitsuru53 requested a review from rafaelfranca June 1, 2021 06:10
@rafaelfranca rafaelfranca merged commit ce18deb into rails:master Jun 1, 2021
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.

2 participants