-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature/custom linters #8
Changes from all commits
1fef014
f471dad
bcd393c
537525e
e8e6b34
2fcb22c
5e3146f
2595854
c816683
00b602b
060a70f
acfa0b2
438d0a3
330bf08
14e715a
2620521
9cf82b7
b5df113
55f28a4
440fd88
0150b1d
92611e2
472d2f5
2c24b72
b7f5f8e
6f30b4a
45fb6fa
7d41260
8c06c88
25d8875
4c7b491
6d95015
f151657
c0611f5
25bee7c
368e857
a251416
3903a21
a0591c4
2785a07
e5a4e96
6a49041
3e52a27
4e6c49a
abaa08e
c229bac
84842c8
d58eaa6
9004908
5776647
a924023
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Gemfile.lock | ||
.bundle | ||
.idea | ||
tmp |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,9 +12,9 @@ This tool lints gherkin files. | |
|
||
run `gherkin_lint` on a list of files | ||
|
||
gherkin_lint FEATURE_FILES | ||
gherkin_lint -f '<wild_card_path>' #default is `features/**/*.feature` | ||
|
||
With `--disable CHECK` or `--enable CHECK` it's possible to disable respectivly enable program wide checks. | ||
With `--disable CHECK` or `--enable CHECK` it's possible to disable respectivly enable program wide checks except when a linter requires additional values to be set in order to be valid. Currently only RequiredTagStartsWith meets this criteria. | ||
|
||
Checks could be disabled using tags within Feature Files. To do so, add @disableCHECK. | ||
Detailed usage within the [disable_tags](https://github.com/funkwerk/gherkin_lint/blob/master/features/disable_tags.feature) feature. | ||
|
@@ -45,6 +45,7 @@ This will mount the current directory within the Gherkin Lint Docker Container a | |
- [missing scenario name](https://github.com/funkwerk/gherkin_lint/blob/master/features/missing_scenario_name.feature) | ||
- [missing test action](https://github.com/funkwerk/gherkin_lint/blob/master/features/missing_test_action.feature) | ||
- [missing verification](https://github.com/funkwerk/gherkin_lint/blob/master/features/missing_verification.feature) | ||
- [required tag starts with](https://github.com/funkwerk/gherkin_lint/blob/master/features/required_tag_starts_with.feature) - disabled by default | ||
- [same tag for all scenarios](https://github.com/funkwerk/gherkin_lint/blob/master/features/same_tag_for_all_scenarios.feature) | ||
- [tag used multiple times](https://github.com/funkwerk/gherkin_lint/blob/master/features/tag_used_multiple_times.feature) | ||
- [too clumsy](https://github.com/funkwerk/gherkin_lint/blob/master/features/too_clumsy.feature) | ||
|
@@ -77,3 +78,6 @@ Install it with: | |
`sudo gem install gherkin_lint` | ||
|
||
After that `gherkin_lint` executable is available. | ||
|
||
## Configuration | ||
If you have a custom configuration you'd like to run on a regular basis instead of passing enable and disable flags through the CLI on every run, you can configure a ```.gherkin_lint.yml``` file that will be loaded on execution. The format and available linters are in [```config/default.yml```](config/default.yml) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would like if there is a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We took a look at this in rubocop and there is an lot of code that makes this work. We figured that this would be an acceptable shortcut for now. You don't have to have .gherkin_lint and the only people who need to know where the default is are gherkin_lint developers. I agree that there needs to be a better way to add new linters, but we didn't want to take that architectural liberty. That said, I'll see if there is a quick and dirty way to solve this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So would adding a CLI option to specify a config file (defaulting to .gherkin_lint.yml) work for you? Auto-gen-config as implemented in rubocop would require quite a few more changes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't checking if there is a Is it okay that if there is a .gherkin_lint.yml and for testing I want to disable a check, that the CLI options override the .gherkin_lint.yml settings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wrote a feature file to demonstrate that this is working as you specified. Is this okay? We are already doing what you asked, I think. I just wrote a test to prove it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
AvoidOutlineForSingleExample: | ||
Enabled: true | ||
AvoidPeriod: | ||
Enabled: true | ||
AvoidScripting: | ||
Enabled: true | ||
BackgroundDoesMoreThanSetup: | ||
Enabled: true | ||
BackgroundRequiresMultipleScenarios: | ||
Enabled: true | ||
BadScenarioName: | ||
Enabled: true | ||
BeDeclarative: | ||
Enabled: true | ||
FileNameDiffersFeatureName: | ||
Enabled: true | ||
MissingExampleName: | ||
Enabled: true | ||
MissingFeatureDescription: | ||
Enabled: true | ||
MissingFeatureName: | ||
Enabled: true | ||
MissingScenarioName: | ||
Enabled: true | ||
MissingTestAction: | ||
Enabled: true | ||
MissingVerification: | ||
Enabled: true | ||
InvalidFileName: | ||
Enabled: true | ||
InvalidStepFlow: | ||
Enabled: true | ||
RequiredTagsStartsWith: | ||
Enabled: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it is a regexp, it could be enabled everytime. so just set it to match everything as Default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment. The idea is that there are multiple types of TagConstraint linters so it didn't make sense to always have an enabled default since, at some point, those linters would easily conflict with each other. |
||
SameTagForAllScenarios: | ||
Enabled: true | ||
TagUsedMultipleTimes: | ||
Enabled: true | ||
TooClumsy: | ||
Enabled: true | ||
TooManyDifferentTags: | ||
Enabled: true | ||
TooManySteps: | ||
Enabled: true | ||
TooManyTags: | ||
Enabled: true | ||
TooLongStep: | ||
Enabled: true | ||
UniqueScenarioNames: | ||
Enabled: true | ||
UnknownVariable: | ||
Enabled: true | ||
UnusedVariable: | ||
Enabled: true | ||
UseBackground: | ||
Enabled: true | ||
UseOutline: | ||
Enabled: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be more specific? I for example have an requirement that I want my tags match a regexp pattern. just allow a few, the others Tags should be valid ticket numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thought about this and decided that there are several kinds of linters that could be implemented (StartsWith, ExactMatch, Regex, Includes) and rather than implement all of them, we implemented what was required for our project along with a Mixin that could be included to create all the others. But rather than try to have this one do everything, we decided to go with the Single Responsibility prinicple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok