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

Option to specify config file with environment variable #255

Merged
merged 5 commits into from
Apr 28, 2020

Conversation

wbadart
Copy link
Contributor

@wbadart wbadart commented Apr 22, 2020

Hey team-

This PR instructs the CLI to check the YAMLLINTRC environment variable for a global config path. For most intents and purposes, this can be achieved with the -c flag, but I found a corner case where the environment variable is useful: I'd like to point my editor and my CI server at the same config file, and it's much easier to specify the variable once in something like a .env file than tweaking how both systems invoke yamllint individually.

I'm not married to the name "YAMLLINTRC"; please let me know if there's a better name (I've been working with pylint lately, so I think I subconsciously borrowed their convention). Also, please let me know if this is PR is the right place to make the corresponding update to the docs.

Thanks!

@wbadart
Copy link
Contributor Author

wbadart commented Apr 22, 2020

Oh hold up, just realized this might be redundant with #119....

EDIT: on a closer reading of 119, I realized that that PR is about putting config data directly into the environment variable, rather than putting a path in the variable (as is the case here).

@coveralls
Copy link

coveralls commented Apr 22, 2020

Coverage Status

Coverage increased (+0.004%) to 97.939% when pulling 0e349d1 on wbadart:master into a54cbce on adrienverge:master.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @wbadart, it looks good.

About the name YAMLLINTRC I agree it's old fashioned, I think the usage of suffix "rc" dates back to before Unix :)

yamllint -h mentions --config-file and --config-data:

  -c CONFIG_FILE, --config-file CONFIG_FILE
                        path to a custom configuration
  -d CONFIG_DATA, --config-data CONFIG_DATA
                        custom configuration (as YAML source)

... so what about YAMLLINT_CONFIG_FILE? (and YAMLLINT_CONFIG_DATA in #119)

tests/test_cli.py Outdated Show resolved Hide resolved
Drops the ancient rc-file convention in favor of YAMLLINT_CONFIG_FILE to
be more consistent with the rest of the documentation.

Uses the `tempfile` module's `NamedTemporaryFile` context manager to
handle the creation and cleanup of temp files (`NamedTemporaryFile` has
a `delete` option, true by default, which removes the file once it's
closed).
@wbadart
Copy link
Contributor Author

wbadart commented Apr 27, 2020

I took your advice and grepped through some of the other tests to see how they're structured. 8657838 should make the new test here much more consistent with those!

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @wbadart!

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.

3 participants