jgrulich added inline comments.

INLINE COMMENTS

> CMakeLists.txt:92
>      qca-qt5
> +    KF5::NetworkManagerQt
> +    KF5::Service

I'm sure you don't need to add all these and why did you remove PUBLIC and 
PRIVATE keywords?

> CMakeLists.txt:111
>  if (WITH_MODEMMANAGER_SUPPORT)
> -    target_link_libraries(plasmanm_editor PUBLIC KF5::ModemManagerQt)
> +    target_link_libraries(plasmanm_editor KF5::ModemManagerQt)
>  endif()

Why removing it?

> CMakeLists.txt:115
>  install(TARGETS plasmanm_editor ${INSTALL_TARGETS_DEFAULT_ARGS})
> -install(FILES plasma-networkmanagement-vpnuiplugin.desktop DESTINATION 
> ${KDE_INSTALL_KSERVICETYPES5DIR})
> +install(FILES plasma-networkmanagement-vpnuiplugin.desktop DESTINATION 
> ${SERVICETYPES_INSTALL_DIR})

This is also an unrelated change.

> simpleiplistvalidator.cpp:27
> +SimpleIpListValidator::SimpleIpListValidator(QObject *parent,
> +                                                       AddressStyle style,
> +                                                       AddressType type)

Fix indentation.

> simpleiplistvalidator.cpp:39
> +            ipv4Style = SimpleIpV4AddressValidator::AddressStyle::WithPort;
> +        m_ipv4Validator = new SimpleIpV4AddressValidator(this, ipv4Style);
> +    }

In both validator constructors (mean also for IPv6 one), you can directly pass 
"style" param from contructor, given both enums seem to be identical.

> CMakeLists.txt:9
> +
> +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O0 -ggdb")
> +

Do we need these flags to be set? Please leak how simply can tested be added to 
CMake

[1] - https://cgit.kde.org/networkmanager-qt.git/tree/autotests/CMakeLists.txt

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15521

To: andersonbruce, jgrulich, pino
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

Reply via email to