hnaderi / lepus

Your principled, purely functional, non-blocking RabbitMQ client for scala, scala js and scala native built on top of fs2
http://lepus.hnaderi.dev/
Apache License 2.0
24 stars 5 forks source link

Consumer Tag #208

Closed rlavolee closed 1 year ago

rlavolee commented 1 year ago

adds the option to define a custom consumer tag or fallback to random tag

rlavolee commented 1 year ago

Hi @hnaderi !

I'm trying to get the PR done but I'm struggling with Mima. Mima is complaining that consumerRaw is not there anymore because I added consumerTag: Option[ConsumerTag] = None in the signature.

method consumeRaw(java.lang.String,Boolean,Boolean,Boolean,scala.collection.immutable.Map)fs2.Stream in class lepus.client.Channel#ConsumingImpl does not have a correspondent in current version

I thought to add the old consumerRaw back as shown below and call the new one in its body.

  def consumeRaw(
      queue: QueueName,
      noLocal: NoLocal = false,
      noAck: NoAck = true,
      exclusive: Boolean = false,
      arguments: FieldTable = FieldTable.empty,
      consumerTag: Option[ConsumerTag] = None
  ): Stream[F, DeliveredMessageRaw]

  final def consumeRaw(
      queue: QueueName,
      noLocal: NoLocal = false,
      noAck: NoAck = true,
      exclusive: Boolean = false,
      arguments: FieldTable = FieldTable.empty
  ): Stream[F, DeliveredMessageRaw] =
    consumeRaw(queue, noLocal, noAck, exclusive, arguments, None)

However the compiler is complaining about overloaded methods because of default arguments.

two or more overloaded variants of method consumeRaw have default arguments

What's the best way to deal with this situation ? Should I remove default args, break binary comp or introduce a new method name ? 🤔

hnaderi commented 1 year ago

@rlavolee

What's the best way to deal with this situation ? Should I remove default args, break binary comp or introduce a new method name ? 🤔

One way would be to add new methods instead of changing the old ones, but it's not worth it and I think we can break the bin-compat here, as the changes are source compatible.

I just bumped up the minor version and pushed it on the PR, so it should be fixed.

hnaderi commented 1 year ago

@rlavolee By the way, are you finished by the PR? or are there other changes you want to make? If everything's OK, we can merge it. I'm just curios about testing a snapshot version on some real projects to see the behavior before merging, which I'll do tomorrow.

rlavolee commented 1 year ago

@hnaderi I'm done with it :) and eager to test it.

hnaderi commented 1 year ago

@rlavolee The snapshot version should be published in a few minutes. Thanks for the contribution! :pray:

rlavolee commented 1 year ago

@hnaderi Your're welcome that was fun. Lepus is great !