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 Steep::RakeTask #995

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Aesthetikx
Copy link

@Aesthetikx Aesthetikx commented Jan 15, 2024

This is to address #667. I am starting this PR as a draft for some discussion, if this seems like a good direction I can rebase, squash, add tests, and documentation.

This adds a Rake::TaskLib subclass similar to other projects like rubocop and rspec, such that a user can add an entry into their own Rakefile to have access to rake tasks for this project. For example,

# Rakefile

require 'steep/rake_task'
Steep::RakeTask.new

would add the following tasks, e.g. the output of bundle exec rake -T now includes:

...
rake steep                    # Run steep check
rake steep:annotations        # Run steep annotations
rake steep:binstub            # Run steep binstub
rake steep:check              # Run steep check
rake steep:checkfile          # Run steep checkfile
rake steep:help               # Run steep help
rake steep:init               # Run steep init
rake steep:langserver         # Run steep langserver
rake steep:project            # Run steep project
rake steep:stats              # Run steep stats
rake steep:validate           # Run steep validate
rake steep:version            # Run steep version
rake steep:watch              # Run steep watch
...

There is also the ability to add some configuration, e.g.

Steep::RakeTask.new do |t|
  t.check.severity_level = :error
  t.watch.verbose
end

Command line arguments can be similarly passed in typical rake fashion:

$ bundle exec rake steep:check[--severity-level=error]

or equivalently

$bundle exec rake steep:check[--severity-level,error]

The rake tasks are automatically generated from Steep::CLI.available_commands, so hopefully this is mostly forwards compatible if the CLI changes. There is some metaprogramming to automatically translate .severity_level = :error into --severity-level error and .verbose into --verbose, which possibly has some room for improvement.

The default steep task is aliased to steep:check, as I feel that this is the most common use case.

I typically have an entry like this at the bottom of Rakefiles for my projects:

task default: %i[spec steep rubocop]

So, this addition should make it easy to add steep to CI pipelines etc. such that simply typing rake will run the tests, typecheck, and lint.

If the CLI command returns nonzero, the rake task exits nonzero, similar to how rubocop and rspec behave. This way, e.g. in the example above, rubocop will not run if steep does not exit cleanly. It will also prevent CI from completing successfully if steep detects any issues.

I was considering changing the default severity level to error as I feel this will be most common -- I did not make that change but I am open to input.

@soutaro
Copy link
Owner

soutaro commented Jan 16, 2024

Thanks!

I guess we don't need steep:languserver steep:checkfile because they won't be used interactively. (But there may be a situation rake steep:langserver can help... I'm pretty sure we don't need steep:checkfile task.)

@soutaro soutaro added this to the Steep 1.7 milestone Jan 16, 2024
@Aesthetikx
Copy link
Author

Thanks!

I guess we don't need steep:languserver steep:checkfile because they won't be used interactively. (But there may be a situation rake steep:langserver can help... I'm pretty sure we don't need steep:checkfile task.)

That makes sense to me -- I am not familiar with all the commands yet, as I am new to steeplife, so I will defer to you. Removed 👍

@Aesthetikx
Copy link
Author

@soutaro Do you have any more feedback or concerns here? If not, I can do some cleanup and add documentation to the README.md about setting up rake tasks, and possibly write a test or two.

@soutaro
Copy link
Owner

soutaro commented Jan 18, 2024

@Aesthetikx Looks good. Having a few tests would be great. 👍

@ParadoxV5
Copy link
Contributor

Some of the commands I’d explicitly write steep … rather than rake steep:…, such as

rake steep:langserver         # Run steep langserver
rake steep:version            # Run steep version
rake steep:watch              # Run steep watch

I say, before we turn the commands into Rake tasks, we need to first figure out what each command is.
(Yes, steep [command] --help is pretty much blank.)

@Aesthetikx
Copy link
Author

@Aesthetikx Looks good. Having a few tests would be great. 👍

I'll try to write tests this weekend :)

Some of the commands I’d explicitly write steep … rather than rake steep:…, such as

rake steep:langserver         # Run steep langserver
rake steep:version            # Run steep version
rake steep:watch              # Run steep watch

I say, before we turn the commands into Rake tasks, we need to first figure out what each command is. (Yes, steep [command] --help is pretty much blank.)

* Is `steep project` a setup command?

* Does `steep annotate` relate to [`manual/annotations.md`](https://github.com/soutaro/steep/blob/master/manual/annotations.md)?

* …

I'll let others chime in here, but I'd just say I agree that for the most part I would continue to just use the steep binary directly for CLI use, but having rake tasks is still nice for CI contexts and other things that integrate with rake.

@ParadoxV5
Copy link
Contributor

I'll let others chime in here, but I'd just say I agree that for the most part I would continue to just use the steep binary directly for CLI use, but having rake tasks is still nice for CI contexts and other things that integrate with rake.

That’s what I’m implying: Some of the command make no sense in CI contexts.
For example, steep watch IIRC runs Steep in a loop that continuously type check modified files,
while steep langserver boots a full-blown LSP for use with IDEs like VSCode.

@Aesthetikx Aesthetikx force-pushed the rake_task branch 5 times, most recently from 8c2bae4 to b74545a Compare February 3, 2024 17:20
@Aesthetikx
Copy link
Author

OK, I cleaned up a bit and added some tests -- I think this is ready for review now 🫖

@Aesthetikx Aesthetikx marked this pull request as ready for review February 3, 2024 17:21
@Aesthetikx Aesthetikx force-pushed the rake_task branch 2 times, most recently from 061f93a to a4efff0 Compare February 3, 2024 17:31
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.

None yet

3 participants