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

ci(travis): reduce matrix down to 5/6 instances (ref: #118) #121

Merged

Conversation

myii
Copy link
Member

@myii myii commented May 19, 2019

  • The selected 5 in a "T"-shaped matrix:
  - 2019.2 (py3): debian-9   fedora-29   opensuse-leap-15
  - 2018.3 (py2):           ubuntu-1604
  - 2017.7 (py2):             centos6
  • Covers each os, each Salt version and both Python versions

I was going to go for centos-7 and fedora-28 instead but centos-6 is a "special" platform, which always throws up interesting situations (including not being systemd-based).

@myii
Copy link
Member Author

myii commented May 19, 2019

So initial results from my fork:

Instances Running time Total time
15 10 min 8 sec 31 min 36 sec
5 4 min 12 sec 13 min 10 sec

Copy link
Member

@aboe76 aboe76 left a comment

Choose a reason for hiding this comment

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

good idea to have different init systems as well, good point.

@myii
Copy link
Member Author

myii commented May 21, 2019

Thanks for the reviews @aboe76 and @n-rodriguez. What do you think @javierbertoli and @daks? Can we get a merge on this so that we can start rolling out this change to the (semantic-release) formulas out there?

@javierbertoli
Copy link
Member

javierbertoli commented May 21, 2019

Wouldn't centos-7 (yum-based installer), cover more cases (centos, redhat, fedora) than fedora (which installs using dnf)? @aboe76 and I suggested using it a few days back.

@myii
Copy link
Member Author

myii commented May 21, 2019

@javierbertoli BTW, you've pinged someone else there, not @aboe76! Anyway, like I said at the top, I was going to do it using centos-7 but I thought it would be good to have a non-systemd system, to cover more scenarios. I'm relaxed either way.

@javierbertoli
Copy link
Member

@javierbertoli BTW, you've pinged someone else there, not @aboe76!

Ooops, fixed the typo 😋

Anyway, like I said at the top, I was going to do it using centos-7 but I thought it would be good to have a non-systemd system, to cover more scenarios. I'm relaxed either way.

Perhaps I was not clear, I meant replacing fedora-29 with centos-7, not dropping centos-6.

The final matrix we suggested was more on the line:

  • 2019.2 (py3): debian-9 centos-7 opensuse-leap-15
  • 2018.3 (py2): ubuntu-1604
  • 2017.7 (py2): centos-6

centos-7 and centos-6 differ enough as to make it reasonable to test them both.

If the argument to add fedora is the same as to add opensuse (a RPM-based distro, different package manager), then I'd say add it to the matrix, ie, 2018.3 (py2) fedora-28?

Perhaps I'm just nitpicking, so I'll 👍 whatever you decide.

@myii
Copy link
Member Author

myii commented May 22, 2019

@javierbertoli This sounds like the ideal solution, 6 in a tree instead of 5 in a T(!):

  - 2019.2 (py3): debian-9   centos7   opensuse-leap-15
  - 2018.3 (py2):     fedora-29   ubuntu-1604
  - 2017.7 (py2):            centos6

…s#118)

* The selected 6 in a "tree"-shaped matrix:
  - 2019.2 (py3): debian-9   centos7   opensuse-leap-15
  - 2018.3 (py2):     fedora-29   ubuntu-1604
  - 2017.7 (py2):            centos6
* Covers each `os`, each Salt version and both Python versions
@myii myii force-pushed the ci/reduce-travis-matrix-instances branch from b2bfd01 to a8834e2 Compare May 22, 2019 00:11
@myii
Copy link
Member Author

myii commented May 22, 2019

So the updated result from my fork (continuing the table above):

Instances Running time Total time
15 10 min 8 sec 31 min 36 sec
5 4 min 12 sec 13 min 10 sec
6 6 min 8 sec 15 min 13 sec

.travis.yml Outdated Show resolved Hide resolved
@myii myii changed the title ci(travis): reduce matrix down to 5 instances (ref: #118) ci(travis): reduce matrix down to 5/6 instances (ref: #118) May 22, 2019
@javierbertoli javierbertoli merged commit dca7888 into saltstack-formulas:master May 22, 2019
@javierbertoli
Copy link
Member

Thanks @myii for your hard work, and all the rest for your feedback!

@myii myii deleted the ci/reduce-travis-matrix-instances branch May 22, 2019 12:27
@myii
Copy link
Member Author

myii commented May 22, 2019

You're welcome, thank you all for your comments, reviews and feedback. Time to start reducing our Travis burdens in the other formulas!

@saltstack-formulas-travis

🎉 This PR is included in version 2.1.12 🎉

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.

6 participants