saltstack-formulas / dhcpd-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
8 stars 57 forks source link

Refactor templates to not use `pillar.get` #46

Closed sticky-note closed 4 years ago

sticky-note commented 4 years 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

44 #45

Describe the changes you're proposing

Refactor templates to not use pillar.get

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

myii commented 4 years ago

@sticky-note Now that #45 is merged, we can look at adjusting the functionality -- please rebase this PR on top of the latest version of this formula.

If you want to replace pillar.get here, then you'll need to reintroduce the block that was taken out of map.jinja during #45. That's if you need the rest of the pillar merged into the map, meaning that the pillar lookup is not enough to work properly here.

myii commented 4 years ago

@sticky-note Just to be clear, if you also merge the rest of the pillar into the map, all of the Travis instances will fail but that's expected, because the yaml_dump InSpec tests will need to be updated to mirror the new entries that are coming in from the pillar. If it doesn't make sense, just make the changes and I'll look at how the InSpec tests need to be updated accordingly.

sticky-note commented 4 years ago

@myii Do we need to do a breaking change then ?

myii commented 4 years ago

@sticky-note Probably not. I've already identified some problems with the existing tests, which is why this PR didn't fail anything! I'll try to get back to you within the day.

myii commented 4 years ago

@sticky-note OK, I've added some commits and force pushed this so that it uses the latest tests. One benefit is showing us how the files have changed before and after this PR.

Once we're finalised with this PR, we can look at rebasing #44 and then fixing all of the tests to confirm everything is working as defined by the tests.

sticky-note commented 4 years ago

@Myii I think ordering is okay

myii commented 4 years ago

@myii I think ordering is okay

OK, merged.

In terms of #44, please go ahead and rebase -- look forward to all of the failing tests! Do you want to fix the tests yourself?

saltstack-formulas-travis commented 4 years ago

:tada: This PR is included in version 0.10.4 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: