pronto icon indicating copy to clipboard operation
pronto copied to clipboard

Way to pass arguments into runners?

Open dorner opened this issue 8 years ago • 14 comments

It would be great if we could pass extra command-line arguments to Pronto runners. I dived into the code but couldn't find an existing way to do this. For example, I'd like to use Pronto locally to auto-correct Rubocop offenses only on my changes before I push my branch. There doesn't seem to be a way to pass extra options to the Rubocop runner.

Did I miss something or does this need to be built in?

dorner avatar Jun 23 '17 17:06 dorner

OK, I just learned about the existence of .rubocop and RUBOCOP_OPTS. -_- Never mind.

dorner avatar Jun 23 '17 20:06 dorner

Hm, nope again. Pronto uses Rubocop classes directly so it doesn't respect either of those things.

dorner avatar Jun 23 '17 20:06 dorner

@dorner there is no way to do it currently. In your example, it would not be enough to implement passing of parameters. We would need to do additional work to give the relevant lines to Rubocop. Do you have any other examples when you'd like to pass parameters to runners? Want to collect the use-cases before trying to figure out how it could be built.

mmozuras avatar Jun 28 '17 18:06 mmozuras

Actually I'd be fine with using it on the files changed rather than the lines changed. Rubocop might report on the wrong lines depending on the cop (e.g. if you added 50 lines to a method and it now fails the "method too long" check, the failure would be above your changes, not inside them).

I can think of a few other reasons to pass arguments to runners. For example, Rubocop has an argument that lets you specify the minimum level where the return value should be non-zero (i.e. default is that warnings will pass, but you can change that so warnings will fail). Currently there's no way to specify that in Pronto.

dorner avatar Jun 29 '17 16:06 dorner

Btw, I'm experimenting now and it looks like autocorrect works just fine out of the box. I've also figured out a way to bypass Pronto and get config directly into the runner, which is by utilizing ConfigFile.to_h. Probably all we need is a method in the base Config class and a method in the base Runner class to retrieve its configuration.

Also, another crucial thing we can't do right now is run Rails cops, because that depends on a command line argument as well.

dorner avatar Jun 30 '17 17:06 dorner

@mmozuras I run into that issue while working on a new runner for https://github.com/mbj/mutant. AFAIK the tool does not provide any configuration via env vars nor config files so I'm looking for a way to pass options to a runner. I was thinking about utilizing @config from runner (using .pronto.yml) but it seems it was meant to only support a known lists of options. What would be a recommended approach?

mknapik avatar Jul 06 '17 14:07 mknapik

@mknapik there is no recommended approach, we have to figure that out 😄.

You can look at how @dorner did it: https://github.com/prontolabs/pronto-rubocop/pulls/dorner. What do you think about this kind of approach for all runners? What would be the alternatives?

/cc @jeroenj @aergonaut @doomspork

mmozuras avatar Jul 15 '17 15:07 mmozuras

These are kind of different issues, no? The first being a bug with how Pronto passed configuration (or failed to) down to Rubocop.

The latter I don't see a problem immediately. If you're writing a new runner wouldn't you handle the ENV/configuration in the runner, translating it to the appropriate arguments for the binary?

doomspork avatar Jul 15 '17 19:07 doomspork

@doomspork not sure if I follow, so let's try again 😄.

@dorner added additional configuration for rubocop specifically to .pronto.yml. See here. It seems to me that @mknapik could do it very similarly.

mmozuras avatar Jul 16 '17 12:07 mmozuras

@mmozuras agreed.

What I meant by my comment was when creating a new runner such as @mknapik is doing, you can access the ENVs in your runner and pass the configuration to the lib you're using; you're not limited by what the library you're using supports.

Code examples are easiest 😀

Here's a runner example:

module Pronto
  class Mutant < Runner
    # ...

    def run
      "mutant #{cli_config}"
    end

    # ... 
  
    def config
      {
        include: ENV['MUTANT_INCLUDE'] || 'default',
        require: ENV['MUTANT_REQUIRE'] || 'default',
        use: ENV['MUTANT_USE'] || 'default'
      }
    end

    def cli_config
      config.map { |k, v| "--#{k} #{v.gsub(',', ' ')}" }.join(' ')
    end
  end
end

If you kick-off pronto with MUTANT_INCLUDE=lib MUTANT_REQUIRE=virtus MUTANT_USE=rspec,Virtus* as your ENVs, you'll generate the command from the Mutant README:

mutant --include lib --require virtus --use rspec Virtus*

That solves @mknapik problem if I'm understanding his question correctly.

doomspork avatar Jul 16 '17 16:07 doomspork

@doomspork @mmozuras Thanks for your feedback! I'll figure out what works best for my case.

mknapik avatar Jul 17 '17 08:07 mknapik

@mknapik When you are done feel free to PR a readme addition mentioning your integration. OT: I intentionally do not support env variables.

mbj avatar Aug 21 '17 19:08 mbj

A unified configuration API for runners would be a nice improvement. The mix of custom runner yamls and environment variables is cumbersome and clutters up the project directory.

I created a pylint runner but the only thing stopping me from contributing it back is implementing some basic configuration.

I would propose creating a way to include runner specific config parameters in the pronto config file and them pass them to the runner on initialization.

Something like:

eslint:
  exclude:
    - 'app/assets/**/*'
  custom:
    cool_feature_toggle: 11

Everything under custom would get passed into the runner itself.

Thoughts?

BobReid avatar Aug 27 '19 19:08 BobReid

@BobReid Mutant was significantly refactored and now has a good configuration API. I also have limited config file support now.

But there is a license change, the new releases will not be under an opensource license anymore. OSS is unsustainable for me.

Still the last OSS release also ships some of the config file support / config API changes.

mbj avatar Oct 09 '19 20:10 mbj