Skip to content

Conversation

@thecodeboss
Copy link

When compiling kafka_ex, there are a bunch of annoying compile warnings that get spit out on later Elixir versions like 1.5 and 1.6.

Firstly, char_list has been deprecated in favour of charlist in many places. For typespecs, we can support both by using [char] instead. For String.to_char_list, we can use apply(String, :to_char_list, [arg]) to stop the warning, and the method exists in all Elixir versions.

Second, Enum.partition is deprecated, so I tweaked to remove the warning.

I also removed some unused arguments, a missing protocol implementation for Enumerable.slice/1, and a typo.


defp remove_stale_brokers(brokers, metadata_brokers) do
{brokers_to_keep, brokers_to_remove} = Enum.partition(brokers, fn(broker) ->
{brokers_to_keep, brokers_to_remove} = apply(Enum, :partition, [brokers, fn(broker) ->

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 94).

case Regex.scan(~r/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/, host) do
[match_data] = [[_, _, _, _, _]] -> match_data |> tl |> List.flatten |> Enum.map(&String.to_integer/1) |> List.to_tuple
_ -> to_char_list(host)
_ -> apply(String, :to_char_list, [host]) # to_char_list is deprecated from Elixir 1.3 onward

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 99).

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 1 possible new issue (including those that may have been commented here).

You can see more details about this review at https://ebertapp.io/github/kafkaex/kafka_ex/pulls/274.

Copy link
Member

@bjhaid bjhaid left a comment

Choose a reason for hiding this comment

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

LGTM, we need to come up with some policy for deprecating older versions of Elixir

def terminate(_, state) do
Logger.log(:debug, "Shutting down worker #{inspect state.worker_name}")
if state.event_pid do
GenEvent.stop(state.event_pid)
Copy link
Member

Choose a reason for hiding this comment

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

This is probably dead code, there should be no use of GenEvent in Kafkaex AFAIK

Copy link
Member

@joshuawscott joshuawscott left a comment

Choose a reason for hiding this comment

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

👍

@joshuawscott joshuawscott merged commit 967749c into kafkaex:master Mar 3, 2018
@joshuawscott
Copy link
Member

Thanks again @thecodeboss

@thecodeboss thecodeboss deleted the remove-compile-warnings branch March 5, 2018 18:37
robotarmy pushed a commit to RAM9/kafka_ex that referenced this pull request Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants