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

also accept .yml (not just .yaml) as configuration extension #232

Closed
matleh opened this issue Apr 8, 2015 · 4 comments
Closed

also accept .yml (not just .yaml) as configuration extension #232

matleh opened this issue Apr 8, 2015 · 4 comments

Comments

@matleh
Copy link

@matleh matleh commented Apr 8, 2015

Most tools I know use ".yml" as extension for YAML files (docker-compose for example).
Invoke currently wants ".yaml".
If Invoke would also accept ".yml" this would spare users who are used to yml an extra thought and file-rename round-trip.

No big deal but nice to have.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented May 5, 2015

I feel like I had a specific reason for not honoring both, but cannot remember offhand what that was (or if I am misremembering). May have been due to an earlier use of a third party lib in the config stuff, for example (which no longer applies).

OK...I looked, the ticket for that is #147, the specific bits where @sophacles and I bantered about it (among other things - just search for "yml" within these comments), are:

Seems like the primary reason besides "it's cleaner to only support one" is that we'd have to declare a separate "load order" for yml vs yaml, should the user have both in the same location for whatever reason, and this was likely to be confusing and/or more complex to implement.

Alternately, we could just explode angrily as per the Zen of Python.

I haven't looked in the config junk basically since that ticket completed...if it's reasonably easy to actually implement this (preferably the explosion option), I would probably be +1 on it, as then the benefits would outweigh the negatives.

@gtback
Copy link
Contributor

@gtback gtback commented May 20, 2015

It seems to be as easy as:

diff --git a/invoke/config.py b/invoke/config.py
index 9ab852e..1ca28fc 100644
--- a/invoke/config.py
+++ b/invoke/config.py
@@ -278,7 +278,7 @@ class Config(DataProxy):
             prefix.
         """
         # Config file suffixes to search, in preference order.
-        self._file_suffixes = ('yaml', 'json', 'py')
+        self._file_suffixes = ('yaml', 'yml', 'json', 'py')

         # Technically an implementation detail - do not expose in public API.
         # Stores merged configs and is accessed via DataProxy.
@@ -557,6 +557,8 @@ class Config(DataProxy):
         with open(path) as fd:
             return yaml.load(fd)

+    _load_yml = _load_yaml
+
     def _load_json(self, path):
         with open(path) as fd:
             return json.load(fd)

(plus documentation changes, of course). I'm happy to make the changes if @bitprophet concurs.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented May 21, 2015

That seems pretty likely, @gtback - I'd probably merge that PR (if it has doc/changelog changes as you mentioned :)) - thanks!

@bitprophet bitprophet changed the title also accept .yml as configuration extension also accept .yml (not just .yaml) as configuration extension May 9, 2016
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Apr 16, 2017

Linked commit was a bit messy, was part impl + changelog. Previous commit included all the tests not discussed herein: 563cecb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.