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

feat: add Rake task #41

Merged
merged 8 commits into from
Jan 5, 2024
Merged

feat: add Rake task #41

merged 8 commits into from
Jan 5, 2024

Conversation

nshki
Copy link
Owner

@nshki nshki commented Jan 5, 2024

Issue

Closes #40.

Overview

As the title states, this PR adds a Rake task for Chusaku. This allows users to use a Rake task to execute Chusaku in addition to the executable it ships with like such:

bin/rake chusaku -- --dry-run

Loading the gem's Rake tasks is a matter of adding a one-liner to a project's Rakefile:

Chusaku.load_tasks

Users will need to update their Gemfile such that Chusaku isn't being required with the require: false flag:

# BAD
gem "chusaku", require: false

# GOOD
gem "chusaku"

This allows local environments to swap Ruby versions using `rbenv`
without accidentally committing updating `.ruby-version`.
@nshki nshki self-assigned this Jan 5, 2024
Copy link
Contributor

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

This looks good to me! I'm about to knock off for the day but I can give it a try locally in my app tomorrow if you like

README.md Outdated
# ...
gem "chusaku", require: false
# ...
gem "chusaku"
Copy link
Contributor

Choose a reason for hiding this comment

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

This means that chusaku will now always be required - while I don't think it'll be a huge problem, given how often chusaku probably won't be required I'd say it's still better to stick with the default of require: false here and encourage people to explicitly require "chusaku" when they need to (such as for loading the rake task).

If you'd like to use Chusaku as a Rake task, add the following line to your `Rakefile`:

```ruby
Chusaku.load_tasks
Copy link
Contributor

Choose a reason for hiding this comment

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

Per above, if you stick with recommending require: false then this should be:

Suggested change
Chusaku.load_tasks
require "chusaku"
Chusaku.load_tasks

To pass flags, pass them like you would from the CLI executable:

```sh
bin/rake chusaku -- --dry-run --exit-with-error-on-annotation
Copy link
Contributor

Choose a reason for hiding this comment

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

From what I've seen working in the Ruby app space generally env variables are used for Rake tasks, but if you've confirmed this works I think it's probably fine to just ship it as-is for now since I don't imagine there'll be a reason to be passing lots of options around, and worse case it can be changed later if you get feedback that it's actually a problem (and that feedback will be very valuable for future)

If users want to use the Rake tasks that ship with the gem, they can
require the main gem class in their `Rakefiles` along with a call that
loads all tasks.
@nshki
Copy link
Owner Author

nshki commented Jan 5, 2024

Thanks for taking a look! I've confirmed that the Rake task works with the same CLI flags and that it works in all the common forms:

bin/rake chusaku -- --dry-run
bundle exec rake chusaku -- --dry-run
rake chusaku -- --dry-run

If you don't mind giving this a whirl on your local machine tomorrow, that would be fantastic.

Copy link
Contributor

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

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

Worked for me after I added the || [] in the task!

Here's my new dev task:

namespace :dev do
  task annotate: %i[environment set_annotation_options] do
    require "chusaku"

    Chusaku.load_tasks

    Rake::Task["chusaku"].invoke
    Rake::Task["annotate_models"].invoke
  end
end

It might be worth reviewing the output later at some point:

❯ bundle exec rake dev:annotate

Chusaku has finished running.
Model files unchanged.

❯ bundle exec rake dev:annotate
Nothing to annotate.
Model files unchanged.

Specifically, removing that leading newline, and changing the "Nothing to annotate" to "Controller files unchanged" or something similar to make it clearer what it's referring to - but I don't think they're critical for this PR and I'm also happy to help do them as a follow up :)

lib/tasks/chusaku.rake Outdated Show resolved Hide resolved
Co-authored-by: Gareth Jones <Jones258@Gmail.com>
@nshki
Copy link
Owner Author

nshki commented Jan 5, 2024

Thank you for the thorough review, @G-Rath! Happy to ship this with the fix you suggested.

Also, thank you for pointing out the weirdness in the output especially when it's next to Annotate's output. Will set aside some time when possible for that, but if you'd like to make a PR, you are more than welcome to!

@nshki nshki merged commit c35763b into main Jan 5, 2024
6 checks passed
@nshki nshki deleted the feat/add-rake-task branch January 5, 2024 19:34
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.

Include a rake task for invoking chusaku
2 participants