Allow to filter runners via config #371

povilasjurcys · 2020-02-25T05:08:49Z

This feature allows to specify which runners will be triggered during pronto run. Might solve #365.

Reason

I use pronto:

on local machine
on CI
as git pre-commit hook
as git pre-push hook

In each of those cases I would like to run different linters. For example I want to run all pronto linters on CI and only fast-enough linters on git pre-commit hook. With this feature I could add PRONTO_SKIP_RUNNERS or PRONTO_RUNNERS env variable in order to control which runners will be triggered.

I know that there is a -r (aka --runner) flag, but:

it does not have --skip-runner options;
it does not work very well, especially with other flags (some times it tries to load file pronto/-r file);
it does not skip runners which where already loaded.

Filtering

Whiteliting runners via config

# .pronto.yml
runners:
  - rubocop
  - reek

This will ensure that only rubocop and reek runners will run.

Blacklisting runners via config

also it's possible to blacklist runners (filter out) like this:

# .pronto.yml
skip_runners:
  - rubocop
  - reek

Filtering via env variable

Filtering can be done with env variables:

# whitelist:
PRONTO_RUNNERS="rubocop reek" pronto run

# blacklist:
PRONTO_SKIP_RUNNERS="rubocop reek" pronto run

Other notable things

I could not resist to do small refactoring in config.rb and runners.rb classes, I hope it's not a big deal. Let me know if I need to open separate ticket just for refactoring.

Also I'm not sure if config structure is good enough. As an alternative I see nested structure for runners like this:

runners:
  only:
    - reek
    - rubocop
  except:
    - rubocop

Would be nice to receive some feedback about it.

povilasjurcys · 2020-03-06T04:57:01Z

@doomspork can you check my PR?

mknapik

Thanks for your contribution!
Skipping a runner for a git hook sounds like a valid use case.

Personally, I like a nested config more.

I'd love to see tests with a runners context with clear 4 cases:

no only nor except
just only
just except
both only and except

.gitignore

lib/pronto/config.rb

lib/pronto/runners.rb

lib/pronto/config.rb

povilasjurcys · 2020-03-31T04:08:32Z

Personally, I like a nested config more.

@mknapik are you talking about this case:

# .pronto.yml
skip_runners:
  - rubocop
  - reek

or something completely different?

I'd love to see tests with a runners context with clear 4 cases:

no only nor except

just only

just except

both only and except

Added more tests. Let me know if there is something else missing.

mknapik · 2020-03-31T08:58:29Z

You've proposed a structure like:

Also I'm not sure if config structure is good enough. As an alternative I see nested structure for runners like this:
runners:
 only:
   - reek
   - rubocop
 except:
   - rubocop

and I think I like it more.
It would also probably require adjusting env vars accordingly (probably PRONTO_RUNNERS_EXCEPT and PRONTO_RUNNERS_ONLY).
@prontolabs/core Does anyone has on opinion on config format?

mknapik

Thanks for the fixes!
Let's wait for other team members @prontolabs/core to weight in on the YML format.

mknapik · 2020-03-31T09:00:14Z

.gitignore

@@ -4,5 +4,4 @@ pkg/*
 .DS_Store
 Gemfile.lock
 coverage
-.history/
 .vscode/


You've missed .vscode (you could amend the commit if you want to)

sorry about that. Fixed and squashed commits

mknapik · 2020-03-31T09:01:01Z

spec/pronto/config_spec.rb

@@ -133,14 +133,33 @@ module Pronto
 describe '#skip_runners' do
 subject { config.skip_runners }

- context 'when there is an entry in the config file' do
+ let(:env_variables) { {} }


ashkulz · 2021-02-18T14:39:15Z

@povilasjurcys can you rebase this PR?

povilasjurcys · 2021-02-19T11:50:52Z

@ashkulz rebased the code 👍

ashkulz · 2021-02-19T12:03:51Z

Hmm, not sure why the CI ran twice and failed for pull_request -- do you have any idea, @dsander?

dsander · 2021-02-19T13:48:38Z

@ashkulz Not super sure to be honest, maybe #398 fixes that. Generally I think it was wrong to use pull_request_target in the action.

ashkulz · 2021-02-19T15:18:03Z

@povilasjurcys can I trouble you to merge/rebase again? GitHub CI failed for 2.4, hopefully #398 fixed that 🤞

ashkulz · 2021-02-19T15:20:00Z

Also, I'd appreciate a change to README.md which documents this change as well 🙂

…unners_via_config

povilasjurcys · 2021-03-02T06:55:11Z

Also, I'd appreciate a change to README.md which documents this change as well

@ashkulz I added two lines in the readme. Let me know if that's enough or not.

can I trouble you to merge/rebase again? GitHub CI failed for 2.4, hopefully #398 fixed that crossed_fingers

Sadly tests for ruby 2.4 is still failing. I highly doubt that it's because of changes in this PR

ashkulz · 2021-03-18T12:02:10Z

I added two lines in the readme. Let me know if that's enough or not.

I think you need to elaborate what happens by default i.e. both the options are not provided, otherwise looks good to me!

Sadly tests for ruby 2.4 is still failing. I highly doubt that it's because of changes in this PR

Sorry to trouble you -- can you merge master again? @dsander made a PR which should fix that.

…unners_via_config

povilasjurcys · 2021-03-23T07:25:42Z

I think you need to elaborate what happens by default i.e. both the options are not provided, otherwise looks good to me!

Added separate table with some top-level properties here. Since it's a new thing, I would like to hear your opinion

povilasjurcys · 2021-05-14T12:00:26Z

@ashkulz can you merge this PR?

jarl-dk · 2021-08-19T12:31:39Z

@ashkulz: It seems like this one can be merged now, right?

ashkulz · 2021-09-06T05:26:59Z

Thanks for the contribution, @povilasjurcys! Sorry for the delay in reviewing 🙈

povilasjurcys requested a review from a team as a code owner February 25, 2020 05:08

mknapik requested changes Mar 26, 2020

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

lib/pronto/config.rb Outdated Show resolved Hide resolved

lib/pronto/runners.rb Show resolved Hide resolved

mknapik reviewed Mar 26, 2020

View reviewed changes

lib/pronto/config.rb Outdated Show resolved Hide resolved

mknapik requested changes Mar 31, 2020

View reviewed changes

Allow to filter runners via config

8e27aee

povilasjurcys force-pushed the allow_to_filter_runners_via_config branch from b88710d to 8e27aee Compare March 31, 2020 09:52

mknapik approved these changes Apr 6, 2020

View reviewed changes

Merge branch 'master' into allow_to_filter_runners_via_config

9897b26

povilasjurcys added 2 commits March 2, 2021 08:49

Update README

132e0cb

Merge remote-tracking branch 'upstream/master' into allow_to_filter_r…

dfb031b

…unners_via_config

povilasjurcys added 3 commits March 23, 2021 09:15

Add short properties description

a060c2e

Merge remote-tracking branch 'upstream/master' into allow_to_filter_r…

1294f25

…unners_via_config

Fix rubocop

9ecf23f

ashkulz merged commit 6b3290e into prontolabs:master Sep 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to filter runners via config #371

Allow to filter runners via config #371

povilasjurcys commented Feb 25, 2020 •

edited

Loading

povilasjurcys commented Mar 6, 2020

mknapik left a comment

povilasjurcys commented Mar 31, 2020

mknapik commented Mar 31, 2020 •

edited

Loading

mknapik left a comment

mknapik Mar 31, 2020

povilasjurcys Mar 31, 2020

mknapik Mar 31, 2020

ashkulz commented Feb 18, 2021

povilasjurcys commented Feb 19, 2021

ashkulz commented Feb 19, 2021

dsander commented Feb 19, 2021

ashkulz commented Feb 19, 2021

ashkulz commented Feb 19, 2021

povilasjurcys commented Mar 2, 2021

ashkulz commented Mar 18, 2021

povilasjurcys commented Mar 23, 2021

povilasjurcys commented May 14, 2021 •

edited

Loading

jarl-dk commented Aug 19, 2021

ashkulz commented Sep 6, 2021

Allow to filter runners via config #371

Allow to filter runners via config #371

Conversation

povilasjurcys commented Feb 25, 2020 • edited Loading

Reason

Filtering

Whiteliting runners via config

Blacklisting runners via config

Filtering via env variable

Other notable things

povilasjurcys commented Mar 6, 2020

mknapik left a comment

Choose a reason for hiding this comment

povilasjurcys commented Mar 31, 2020

mknapik commented Mar 31, 2020 • edited Loading

mknapik left a comment

Choose a reason for hiding this comment

mknapik Mar 31, 2020

Choose a reason for hiding this comment

povilasjurcys Mar 31, 2020

Choose a reason for hiding this comment

mknapik Mar 31, 2020

Choose a reason for hiding this comment

ashkulz commented Feb 18, 2021

povilasjurcys commented Feb 19, 2021

ashkulz commented Feb 19, 2021

dsander commented Feb 19, 2021

ashkulz commented Feb 19, 2021

ashkulz commented Feb 19, 2021

povilasjurcys commented Mar 2, 2021

ashkulz commented Mar 18, 2021

povilasjurcys commented Mar 23, 2021

povilasjurcys commented May 14, 2021 • edited Loading

jarl-dk commented Aug 19, 2021

ashkulz commented Sep 6, 2021

povilasjurcys commented Feb 25, 2020 •

edited

Loading

mknapik commented Mar 31, 2020 •

edited

Loading

povilasjurcys commented May 14, 2021 •

edited

Loading