AddrType enums in roc_address

[alexandremgo]

Discussed in #300 : Moving AddrType enum to roc_address and creation of set_host_port(type, ...) and set_miface(type, ...).

I have some questions: 1) Should we keep set_host_port_ipv4(...), set_host_port_ipv6(...), set_miface_ipv4(...), and set_miface_ipv6(...) ?

2) Should we replace all the occurrences of those 4 methods by set_host_port(type, ...) and set_miface(type, ...) ?

3) Should we change version() to return the AddrType enum ?

[gavv]

Should we keep set_host_port_ipv4(...), set_host_port_ipv6(...), set_miface_ipv4(...), and set_miface_ipv6(...) ?

Let's drop them.

Should we replace all the occurrences of those 4 methods by set_host_port(type, ...) and set_miface(type, ...) ?

I think yes.

Should we change version() to return the AddrType enum ?

Also yes.

[gavv]

Two stylistic notes:

• Enum values should be camel-case starting from capital letter (unknown -> Unknown). And we usually use common prefix, ending with an underscore, for all enum values, so it'd be for example Family_Unknown, Family_IPv4, etc.

• Private methods should end with underscore, like: foo_().

[gavv]

For switch-related compiler warning (currently in travis), we usually use this trick:

    switch ((unsigned)args.color_arg) {
    case color_arg_auto:
        core::Logger::instance().set_colors(
            core::colors_available() ? core::ColorsEnabled : core::ColorsDisabled);
        break;

    case color_arg_always:
        core::Logger::instance().set_colors(core::ColorsMode(core::ColorsEnabled));
        break;

    case color_arg_never:
        core::Logger::instance().set_colors(core::ColorsMode(core::ColorsDisabled));
        break;

    default:
        break;
    }

The cast and default branch are needed to shut up compilers. Otherwise some versions of gcc/clang will produce controversial warnings :/ I don't remember exactly, but probably you'll see those warning on travis.

[gavv]

BTW, another hint (if you didn't know): you can run CI builds locally, it may make the development cycle faster: https://roc-project.github.io/roc/docs/development/continuous_integration.html#run-locally

[gavv]

LGTM! Can we merge this?

[gavv]

I recently pushed roc_peer tests which also use set_host_port:

• https://github.com/roc-project/roc/blob/807ec5fa06a8b630d42d5ce39f970d3741cf6d8a/src/tests/roc_peer/test_receiver.cpp#L45

• https://github.com/roc-project/roc/blob/807ec5fa06a8b630d42d5ce39f970d3741cf6d8a/src/tests/roc_peer/test_sender.cpp#L45

Could you please rebase and update them too?

[alexandremgo]

I recently pushed roc_peer tests which also use set_host_port:

* https://github.com/roc-project/roc/blob/807ec5fa06a8b630d42d5ce39f970d3741cf6d8a/src/tests/roc_peer/test_receiver.cpp#L45

* https://github.com/roc-project/roc/blob/807ec5fa06a8b630d42d5ce39f970d3741cf6d8a/src/tests/roc_peer/test_sender.cpp#L45

Could you please rebase and update them too?

Yep, done ;)

[gavv]

Thanks!

[gavv]

FYI: 70d3c5044391a75db1b517ced9771b9bf6b19262