Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

bump to amqp 1.0.x series #16

Closed
wants to merge 1 commit into from
Closed

Conversation

dch
Copy link
Contributor

@dch dch commented Jun 7, 2018

Description

The current amqp dependency is pretty old, let's keep up to date with rabbitmq/pivotal upstream.

  • update .gitignore after log testing
  • bump mix.* as required
  • correct minor spelling issues
Randomized with seed 659912
----------------
COV    FILE                                        LINES RELEVANT   MISSED
 81.5% lib/consumer.ex                               377       65       12
100.0% lib/message.ex                                 22        1        0
  0.0% lib/processor.ex                                6        0        0
 84.2% lib/publisher.ex                              173       19        3
100.0% lib/rabbit_case.ex                             75        1        0
[TOTAL]  82.6%
----------------

Superficial tests seem to pass on FreeBSD 12.0-CURRENT amd64 + RabbitMQ 3.7.5.

Checklist

  • I have added unit tests to cover my changes.
  • I have improved the code quality of this repo. (refactoring, or reduced number of static analyser issues)
  • I have updated the documentation accordingly

- update .gitignore after log testing
- bump mix.* as required
@dch dch requested review from mkorszun and vorce as code owners June 7, 2018 22:23
@vorce
Copy link
Collaborator

vorce commented Jun 8, 2018

Hey @dch - thanks for the PR. Will have a look asap.

Copy link
Collaborator

@mkorszun mkorszun left a comment

Choose a reason for hiding this comment

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

@dch Looks good, just couple of small things:

  • Let's disable lagger logs so we do not see them while running tests:
config :lager, handlers: [level: :critical]

Without this we would see:

13:32:25.498 [error] Supervisor 'Elixir.Logger.Supervisor' had child 'Elixir.Logger.ErrorHandler' started with 'Elixir.Logger.Watcher':start_link({error_logger,'Elixir.Logger.ErrorHandler',{true,false,500}}) at <0.209.0> exit with reason normal in context child_terminated
13:32:25.545 [info] Application lager started on node nonode@nohost
13:32:25.551 [info] Application xmerl started on node nonode@nohost
13:32:25.558 [info] Application ranch started on node nonode@nohost
13:32:25.560 [info] Application ranch_proxy_protocol started on node nonode@nohost
13:32:25.562 [info] Application recon started on node nonode@nohost
13:32:25.562 [info] Application rabbit_common started on node nonode@nohost
13:32:25.568 [info] Application amqp_client started on node nonode@nohost
13:32:25.568 [info] Application amqp started on node nonode@nohost
13:32:25.569 [info] Application gen_rmq started on node nonode@nohost
13:32:25.584 [info] Application ex_unit started on node nonode@nohost
  • After upgrading credo we need to ensure we install poison for dev as well:

from {:poison, "~> 3.1", only: :test} to {:poison, "~> 3.1", only: [:dev, :test]}

Currently it fails with:

Dependencies have diverged:
* poison (Hex package)
  the :only option for dependency poison

  > In mix.exs:
    {:poison, "~> 3.1", [env: :prod, repo: "hexpm", hex: "poison", only: :test, manager: :mix]}

  does not match the :only option calculated for

  > In deps/credo/mix.exs:
    {:poison, ">= 0.0.0", [only: :dev, env: :prod, hex: "poison", repo: "hexpm", optional: false]}

  Ensure you specify at least the same environments in :only in your dep
** (Mix) Can't continue due to errors on dependencies

@mkorszun
Copy link
Collaborator

mkorszun commented Jun 8, 2018

We will also need to write similar migration notice -> https://github.com/pma/amqp/wiki/Upgrade-from-0.X-to-1.0#lager

@mkorszun
Copy link
Collaborator

Addressed in #22

@mkorszun mkorszun closed this Jun 26, 2018
@mkorszun
Copy link
Collaborator

mkorszun commented Jul 2, 2018

@dch we have just release new version with amqp upgraded to 1.0.3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants