Skip to content

added support for .yaml along with .yml#85

Merged
epcim merged 5 commits intosalt-formulas:developfrom
pranavgupta1234:extend_file_extension
Jun 19, 2019
Merged

added support for .yaml along with .yml#85
epcim merged 5 commits intosalt-formulas:developfrom
pranavgupta1234:extend_file_extension

Conversation

@pranavgupta1234
Copy link
Copy Markdown

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

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

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

pranavgupta1234 commented May 2, 2019

Yes, its unnecessary. Refactored.

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

@epcim any update ?

@m-d-johnson
Copy link
Copy Markdown

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

@pranavgupta1234
Copy link
Copy Markdown
Author

@epcim ping.

@epcim
Copy link
Copy Markdown
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