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

Fix logging, minor bugs & kitchen testing #102

Merged
merged 21 commits into from
Jul 19, 2019

Conversation

alxwr
Copy link
Member

@alxwr alxwr commented May 24, 2019

  • Based on Add kitchen and fix logging #98: (Thanks, @tigpas!)

    • fix logging
    • added kitchen testing
  • implemented testing based on template formula

  • several minor fixes of bugs found by CI

  • exclude Fedora 29 from CI (for now)

@n-rodriguez I plan to close #98 in favor of this PR.

@alxwr alxwr requested a review from n-rodriguez May 24, 2019 22:34
@alxwr alxwr self-assigned this May 24, 2019
@alxwr alxwr mentioned this pull request May 24, 2019
@alxwr
Copy link
Member Author

alxwr commented May 24, 2019

Oh, I missed #101. Will merge it and rebase against it.

@alxwr
Copy link
Member Author

alxwr commented May 24, 2019

I'll fix the commit messages...

@alxwr alxwr force-pushed the tigpas-add-kitchen-fix-log branch from 178117f to 499444a Compare May 24, 2019 22:56
Copy link
Member

@n-rodriguez n-rodriguez left a comment

Choose a reason for hiding this comment

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

thank you!

kitchen.yml Outdated
'*':
- openvpn
pillars_from_files:
openvpn.sls: test/config/pillars.sls
Copy link
Member

Choose a reason for hiding this comment

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

openvpn/map.jinja Outdated Show resolved Hide resolved
pillar.example Outdated Show resolved Hide resolved
test/integration/default/controls/services_spec.rb Outdated Show resolved Hide resolved
test/integration/default/controls/config_spec.rb Outdated Show resolved Hide resolved
test/integration/default/controls/config_spec.rb Outdated Show resolved Hide resolved
test/integration/default/controls/services_spec.rb Outdated Show resolved Hide resolved
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.

From a semantic-release perspective (if that's the plan), there are a few more necessary changes:

  1. Needs a FORMULA file.
  2. Needs the pre-commit_semantic-release.sh file.
  3. Needs the docs/CONTRIBUTING.rst file (with the last two sections removed).
  4. Needs the updates to the docs/README.rst file.

For these last two, it's easier to get this from one of the latest semantic-release conversions, that are shown on saltstack-formulas/template-formula#21. For example:

If semantic-release wasn't the plan, then please ignore this! This can be used for a separate PR instead.

.kitchen.local.yml
.cache
junit-*.xml
__pycache__
Copy link
Member

Choose a reason for hiding this comment

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

Since this file is being touched, do you merging it with the one in template-formula? The only difference is junit-*.xml. Otherwise, the rest can be pulled in from there.

@myii
Copy link
Member

myii commented May 25, 2019

By the way, thanks to you both @tigpas and @alxwr for this contribution!

@alxwr
Copy link
Member Author

alxwr commented May 25, 2019

@myii @n-rodriguez Thanks for your reviews and hints. I'll update the PR as soon as I find some time to do so!

@n-rodriguez
Copy link
Member

Hi there! Any news?

@myii
Copy link
Member

myii commented Jun 28, 2019

@alxwr When you get around to modifying this, could you please update kitchen+travis to use the new develop images, in the same way as the template-formula? Thanks.

@alxwr alxwr force-pushed the tigpas-add-kitchen-fix-log branch from b678ce2 to dd327aa Compare July 13, 2019 17:31
@alxwr
Copy link
Member Author

alxwr commented Jul 13, 2019

@n-rodriguez Hello there! The past weeks were challenging. I hope my fixes match your remarks. :-) Please review!

@myii Yes, I want to implement semantic versioning. Please review those changes!

Thanks to both of you for your patience!

@alxwr alxwr force-pushed the tigpas-add-kitchen-fix-log branch from dd327aa to 9044a92 Compare July 13, 2019 18:12
@n-rodriguez
Copy link
Member

@alxwr great work 👍

@alxwr
Copy link
Member Author

alxwr commented Jul 13, 2019

@n-rodriguez Great review! :-)

@myii
Copy link
Member

myii commented Jul 16, 2019

@alxwr Is this PR ready to be merged? Everything is in place from a semantic-release perspective. There are a few minor points but they can be covered in a future PR; I don't want to hold this up for any longer. Actually, one thing that is important to provide at this stage is bin/kitchen, since it's mentioned in the README.

@alxwr
Copy link
Member Author

alxwr commented Jul 16, 2019

@myii: If you tell me how to do it, I'll add the missing parts.
The PR is ready from my perspective, I'm just waiting for @n-rodriguez' OK. :-)

@myii
Copy link
Member

myii commented Jul 16, 2019

@alxwr To be honest, I'm working on a method of standardising all of the approximately 30 semantic-release formulas. It will be a semi-automatic process, which will save the time of having to conduct lengthy reviews! If you can add bin/kitchen to this PR, that would be great.

@myii
Copy link
Member

myii commented Jul 16, 2019

@n-rodriguez We need #102 (comment) in the template-formula as well... would you care to fix it or shall I add it to the never-ending SaltStack Formulas list?!

@alxwr
Copy link
Member Author

alxwr commented Jul 16, 2019

@myii That's great news! Thanks! Added bin/kitchen.

@myii
Copy link
Member

myii commented Jul 17, 2019

@alxwr Happens to us all! The last commit got caught out by commitlint:

⧗   input: Added bin/kitchen
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

@alxwr alxwr force-pushed the tigpas-add-kitchen-fix-log branch from 9931f60 to 15bea7b Compare July 17, 2019 10:21
@alxwr
Copy link
Member Author

alxwr commented Jul 17, 2019

@myii I should know better by now... Fixed the commit.

@myii
Copy link
Member

myii commented Jul 18, 2019

@alxwr Shall we go ahead and merge? You made the fix that @n-rodriguez requested.

@alxwr
Copy link
Member Author

alxwr commented Jul 19, 2019

@myii That's ok for me. :-)

@myii myii merged commit 820c15d into saltstack-formulas:master Jul 19, 2019
@myii
Copy link
Member

myii commented Jul 19, 2019

Merged. Thanks @alxwr and @n-rodriguez.

@saltstack-formulas-travis

🎉 This PR is included in version 0.13.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants