search

salt-formulas / reclass

A recursive external node classifier for automation tools like Ansible, Puppet, and Salt
Other
53 stars 23 forks source link

added support for .yaml along with .yml #85

Closed pranavgupta1234 closed 5 years ago

pranavgupta1234 commented 5 years ago

Fixes https://github.com/salt-formulas/reclass/issues/83 As discussed before adding support for arbitrary extensions is not a good idea but we can defintely add support for .yaml extension.

As fnmatch was doing simple extension matching so I removed it and used simple endswith function as I was unable to find cleaner API which can utilize fnmatch.filter(...) with list of patterns. Let me know your thoughts on this.

AndrewPickford commented 5 years ago

It's nice simple change and I don't see any issue in supporting a .yaml file extension as well as .yml (it's even on the original reclass todo list). But could you update the yaml_git storage type as well, otherwise I think the change breaks yaml_git storage.

pranavgupta1234 commented 5 years ago

@AndrewPickford thanks for the review. Please have a look now.

AndrewPickford commented 5 years ago

Did a quick check of the yaml_git change and it works fine for me. One question: what's the reason for wrapping file.name in a str()? It works fine without for me.

pranavgupta1234 commented 5 years ago

Yes, its unnecessary. Refactored.

AndrewPickford commented 5 years ago

@epcim Could you merge in this pull request. Once that's done I'll do a little refactoring and pull out the separate definitions of FILE_EXTENSION and then put them somewhere central.

pranavgupta1234 commented 5 years ago

@epcim any update ?

m-d-johnson commented 5 years ago

This would be useful for me to have - any idea if this is likely to be merged?

pranavgupta1234 commented 5 years ago

@epcim ping.

epcim commented 5 years ago

Sorry for delay. Merged