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(latest.sls): use Bintray repo on Debian #53

Merged
merged 1 commit into from
Nov 14, 2019
Merged

fix(latest.sls): use Bintray repo on Debian #53

merged 1 commit into from
Nov 14, 2019

Conversation

sylvainfaivre
Copy link
Contributor

@sylvainfaivre sylvainfaivre commented Nov 13, 2019

The rabbitmq.com debian repo is outdated (no update since January 2018)
The Bintray repo provides Erlang and RabbitMQ packages, see https://www.rabbitmq.com/install-debian.html

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

None

Describe the changes you're proposing

Use Bintray repo instead of outdated rabbitmq.com repo

Pillar / config required to test the proposed changes

None

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@myii
Copy link
Member

myii commented Nov 13, 2019

Thanks for this PR, @sylvainfaivre. Just an initial suggestion about the commit message.

https://travis-ci.com/saltstack-formulas/rabbitmq-formula/jobs/256187697#L346-L350

✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]
✖   found 2 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

So perhaps it can be changed to:

-Use Bintray repo on Debian
+fix(latest.sls): use Bintray repo on Debian

@myii
Copy link
Member

myii commented Nov 13, 2019

Another problem we have here (unrelated to this PR) is that the rabbitmq.latest state isn't being run in the CI. So this PR isn't being tested so far. Perhaps it would be worth merging a PR for that first, so that this PR can benefit from that as well.

@myii
Copy link
Member

myii commented Nov 13, 2019

Tested adding a latest suite, before and after this PR:

We can skip around the common failures for the time being. This PR looks like it is appropriate.

The rabbitmq.com debian repo is outdated (no update since January 2018)
The Bintray repo provides Erlang and RabbitMQ packages, see https://www.rabbitmq.com/install-debian.html
@sylvainfaivre
Copy link
Contributor Author

OK I edited the commit title, do you want to merge your changes to the tests before merging this PR ? Anything else ?

Copy link
Member

@daks daks left a comment

Choose a reason for hiding this comment

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

LGTM

@myii
Copy link
Member

myii commented Nov 14, 2019

OK I edited the commit title, do you want to merge your changes to the tests before merging this PR ? Anything else ?

@sylvainfaivre Thanks for making the change. Unfortunately, need a slight adjustment to pass the commitlint, so that this makes a new release with semantic-release (see the commit message formatting section of CONTRIBUTING for more details):

-fix(latest.sls): Use Bintray repo on Debian
+fix(latest.sls): use Bintray repo on Debian

Otherwise, this looks good to go.

@myii myii changed the title fix(latest.sls): Use Bintray repo on Debian fix(latest.sls): use Bintray repo on Debian Nov 14, 2019
@myii myii self-requested a review November 14, 2019 14:42
@myii myii merged commit b50f347 into saltstack-formulas:master Nov 14, 2019
@myii
Copy link
Member

myii commented Nov 14, 2019

@sylvainfaivre It's OK, I've made the change during the merge process, so the release should be created as usual. Also noticed this is your first ever contribution -- welcome to GitHub!

Thanks for the review, @daks.

@saltstack-formulas-travis

🎉 This PR is included in version 0.15.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

myii added a commit to myii/ssf-formula that referenced this pull request Nov 17, 2019
myii added a commit to myii/ssf-formula that referenced this pull request Nov 17, 2019
myii pushed a commit to myii/ssf-formula that referenced this pull request Nov 19, 2019
# [1.57.0](v1.56.1...v1.57.0) (2019-11-19)

### Continuous Integration

* **travis:** opt-in to `dpl v2` to complete build config validation ([1af7a81](1af7a81)), closes [/github.com/travis-ci/dpl/issues/1138#issuecomment-554988130](https://github.com//github.com/travis-ci/dpl/issues/1138/issues/issuecomment-554988130)

### Features

* **rabbitmq:** add `latest` suite ([8df7a31](8df7a31)), closes [/github.com/saltstack-formulas/rabbitmq-formula/pull/53#issuecomment-553480289](https://github.com//github.com/saltstack-formulas/rabbitmq-formula/pull/53/issues/issuecomment-553480289)
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