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

added support for .yaml along with .yml #85

Merged
merged 5 commits into from
Jun 19, 2019

Conversation

pranavgupta1234
Copy link

@pranavgupta1234 pranavgupta1234 commented May 2, 2019

Fixes #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
Copy link
Collaborator

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
Copy link
Author

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

@AndrewPickford
Copy link
Collaborator

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
Copy link
Author

pranavgupta1234 commented May 2, 2019

Yes, its unnecessary. Refactored.

@AndrewPickford
Copy link
Collaborator

@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
Copy link
Author

@epcim any update ?

@m-d-johnson
Copy link

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

@pranavgupta1234
Copy link
Author

@epcim ping.

@epcim
Copy link
Member

epcim commented Jun 19, 2019

Sorry for delay. Merged

@epcim epcim merged commit b8de2fb into salt-formulas:develop Jun 19, 2019
MatteoVoges pushed a commit to neXenio/reclass that referenced this pull request Apr 4, 2023
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.

allowing custom extensions (.yml and yaml) for more flexibility
4 participants