saltstack-formulas / template-formula

SaltStack formula template filled with dummy content
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
119 stars 85 forks source link

refactor: moving jinja to separate dir for cleaner layout #252

Open vveliev-tc opened 1 year ago

vveliev-tc commented 1 year ago

PR progress checklist (to be filled in by reviewers)


What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Describe the changes you're proposing

Simply moving jinja files to separate folder to make bit more cleaner root folder. (jinja may scare new contributors ;) )

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

vveliev-tc commented 1 year ago

I was actually not sure about the name for the folder, since there is a git ignore for lib https://github.com/saltstack-formulas/template-formula/blob/master/.gitignore#L18

should it be libs/jinja/map.jinja or just libs/map.jinja ?

baby-gnu commented 1 year ago

I was actually not sure about the name for the folder, since there is a git ignore for lib https://github.com/saltstack-formulas/template-formula/blob/master/.gitignore#L18

should it be libs/jinja/map.jinja or just libs/map.jinja ?

I personally prefer libs/map.jinja.

Regards.

daks commented 1 year ago

If we all three agree for /libs directory, let's go for it! :)

dafyddj commented 1 year ago

This might be a minor detail, but I would go for lib myself. The fact that lib is in .gitignore seems to be by accident rather than by design.

vveliev-tc commented 1 year ago

let agree on the style before I change it again :).

lib/s - maybe confused with formula libs IMO. altho is usually under package/install for formulas jinja - indicates that it's jinja files

I have no strong opinion on any of them as long as its not in formula root directory

baby-gnu commented 1 year ago

let agree on the style before I change it again :).

lib/s - maybe confused with formula libs IMO. altho is usually under package/install for formulas jinja - indicates that it's jinja files

I have no strong opinion on any of them as long as its not in formula root directory

Actually, there is only a single formula with a libs/ directory, hadoop-formula.

This formula is not updated since 2018 so I really advocate to don't bother and use libs/ ;-)

Regards.

baby-gnu commented 1 year ago

Thanks :)

Now, I'm wondering how to translate this to ssf-formula :sweat_smile:

baby-gnu commented 1 year ago

Hello, looking at this for another formula, I wonder if the per formula post-map.jinja shouldn't be under parameters/ instead of libs/.

This post-map.jinja is more about specific data manipulations instead of a generic library.

As a user of a formula, I'll give more attention to files under parameters/ to understand a formula rather than under libs/.

Only 2 formulas use this post-map.jinja for now:

daks commented 1 year ago

Thanks :)

Now, I'm wondering how to translate this to ssf-formula sweat_smile

I have no idea neither. This is a problem because I think only Imran knows how it works

vveliev-tc commented 1 year ago

Hello, looking at this for another formula, I wonder if the per formula post-map.jinja shouldn't be under parameters/ instead of libs/.

This post-map.jinja is more about specific data manipulations instead of a generic library.

As a user of a formula, I'll give more attention to files under parameters/ to understand a formula rather than under libs/.

Only 2 formulas use this post-map.jinja for now:

I'm not sure to be honest sounds like parameters/ will be better place for post-map.jinja as jinja files will be expected under parameters as well.

does it need to be resolved for this PR?

baby-gnu commented 1 year ago

I'm not sure to be honest sounds like parameters/ will be better place for post-map.jinja as jinja files will be expected under parameters as well.

I'm not sure to understand your point 🤔

To me:

does it need to be resolved for this PR?

This PR will be a starting point for new standard accross formulas, I think we should take time to make it clean and permit to everybody to give their points.

Regards.

vveliev-tc commented 1 year ago

@baby-gnu I think we are on the same page, I have updated map.jinja to reflect that.

Is there anythinb else I should change?