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

Feature: Ability to configure enabled violations #31

Open
Levivb opened this issue Jan 12, 2019 · 1 comment
Open

Feature: Ability to configure enabled violations #31

Levivb opened this issue Jan 12, 2019 · 1 comment
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Levivb
Copy link

Levivb commented Jan 12, 2019

Feature request

Having a linter is cool and all, but the use case of each developer changes from case to case. I understand how a lot of these violations make sense, but there might be cases at which a certain violation should be disabled due to the implementation of the dotenv loader.

For example, see this release note of the php dot env loader: https://github.com/vlucas/phpdotenv/releases/tag/v3.0.0 which advertises multiline comments. According to https://github.com/vlucas/phpdotenv/blob/master/src/Loader.php#L16 it will only work when wrapping the value in quotes.
In older versions of the loader it would even throw an exception when working with a value with spaces not wrapped in quotes: https://github.com/vlucas/phpdotenv/blob/2.5/src/Loader.php#L262

Another example, albeit less substantiated, having duplicate keys in a dotenv would be possible when the loader provides functionality similar to parsing .ini files.
A valid .ini example:

[DATABASE]
database = mysqli://username:password@localhost/my_database
[ADMIN_DATABASE]
database = mysqli://admin:adminpassword@localhost/my_database

Perhaps a silly example but I hope you understand my point :P

What's wrong

There's currently no way to enable/disable specific violations. As of this moment, I can't implement this package in our pipeline due to the disallowed quoted values. For other developers, it could be any of the other violations which would prevent them from using this linter.

How is that should be

Having a way to configure the violations which are applicable to the developer would make this package a whole lot more useful since it's adoption would be a lot easier and flexible

I suggest, besides perhaps supporting the configuration of violations via cli arguments, to be able to provide a config file containing the configuration for the linter to use.
A good example would be the php code sniffer which supports an xml file containing the full configuration for the code sniffer command to use.
Another syntax like yaml or json would be fine as well of course.

And as icing on the cake you could even let developers configure whether an error or a warning is issued for each violation. This would make it even more useful in automated pipelines where errors are not allowed but warnings won't halt the pipeline. For example: quoted strings aren't really a big deal, so if a developer wants to, that could be a warning. While having duplicate name in your dotenv is a big no go. But this could be a separate feature request which would be dependent on this feature request.

Let me know what you think :)

As a sidenote, this feature request might depend on this feature: Test each unique violation #13

@sobolevn
Copy link
Member

@Levivb absolutely agree with you. We need a mechanism to configure this tool.

First release was intentionally made as simple as possible. But, configuration is on a roadmap.

@sobolevn sobolevn added enhancement New feature or request help wanted Extra attention is needed labels Jan 15, 2019
@sobolevn sobolevn mentioned this issue Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants