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.

Sign up for GitHub

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

Jump to bottom

Add tofs, use pre-salted images #7

Merged
merged 2 commits into from
May 21, 2019
Merged

Add tofs, use pre-salted images #7

merged 2 commits into from
May 21, 2019

Conversation

n-rodriguez
Copy link
Member

No description provided.

@n-rodriguez n-rodriguez requested a review from myii May 14, 2019 20:42
@myii
Copy link
Member

myii commented May 15, 2019

@n-rodriguez Sorry for the delay, time is is short and this is a large PR! I've done a quick eyeball check and I've got a few suggestions. Let me at least provide these so that the PR can keep moving forward.

myii
myii requested changes May 15, 2019
View reviewed changes
Copy link
Member

@myii myii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a brilliant, comprehensive PR. Just a few recommendations and a few suggestions.

ufw/config/applications.sls Outdated Show resolved Hide resolved
ufw/config/applications.sls Show resolved Hide resolved
ufw/config/file.sls Outdated Show resolved Hide resolved
ufw/config/file.sls Outdated Show resolved Hide resolved
ufw/config/applications.sls Outdated Show resolved Hide resolved
ufw/service/enable.sls Outdated Show resolved Hide resolved
ufw/service/reload.sls Outdated Show resolved Hide resolved
ufw/service/running.sls Outdated Show resolved Hide resolved
ufw/service/running.sls Outdated Show resolved Hide resolved
ufw/files/default/ufw.sysctl.tmpl.jinja Show resolved Hide resolved
@myii
Copy link
Member

myii commented May 15, 2019

@n-rodriguez As I alluded to in one of the comments above, are we at the level of BREAKING CHANGE here? I usually compare the map before and after a PR and this one has changed significantly, so I can't tell how much it has changed. Is this PR 100% backwards compatible? If not, then BREAKING CHANGE will be needed (and my relevant comments above can be considered for inclusion).

@n-rodriguez n-rodriguez requested a review from myii May 18, 2019 20:48
@myii
Copy link
Member

myii commented May 18, 2019

@n-rodriguez I'll look at this very soon. While you're waiting for me in the meantime, would you mind selecting 5 Travis instances to use and commenting out the rest? You don't need to modify kitchen.yml, it's good to still have everything there for local testing. See saltstack-formulas/template-formula#118 (comment).

@myii
Copy link
Member

myii commented May 18, 2019

@n-rodriguez Notice @aboe76's new comment: saltstack-formulas/template-formula#118 (comment). Perhaps have the three main ones from 2019.2, one from 2018.3 and the last one from 2017.7?

@myii myii mentioned this pull request May 18, 2019
Open
myii
myii requested changes May 18, 2019
View reviewed changes
ufw/config/file.sls Outdated Show resolved Hide resolved
ufw/files/default/ufw.sysctl.tmpl.jinja Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@myii
Copy link
Member

myii commented May 18, 2019

@n-rodriguez Almost there, sorry for making you jump through hoops but this is a massive PR!

@n-rodriguez n-rodriguez requested a review from myii May 21, 2019 19:48
@myii myii merged commit 4a9b7e7 into saltstack-formulas:master May 21, 2019
@myii
Copy link
Member

myii commented May 21, 2019

@n-rodriguez Merged, thanks for you work and patience! In terms of the final matrix, what do you think of switching this for the "T"-shaped matrix proposed in saltstack-formulas/template-formula#121?

@saltstack-formulas-travis
Copy link

🎉 This PR is included in version 0.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@myii myii mentioned this pull request Jun 11, 2019
Open
@n-rodriguez n-rodriguez deleted the wip/ci branch August 25, 2019 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aboe76 aboe76 aboe76 approved these changes

@myii myii myii approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@n-rodriguez @myii @saltstack-formulas-travis @aboe76