pranavgade marked an inline comment as done.
pranavgade added inline comments.

INLINE COMMENTS

> jgrulich wrote in dcbsetting.cpp:31
> You can initialize also all the lists here, because they have default values 
> as well.

how do i do that?
i am getting this error:
error: class ‘NetworkManager::DcbSettingPrivate’ does not have any field named 
‘setPriorityFlowControl’

> jgrulich wrote in dcbsetting.cpp:197
> If you initialize the list at the beginning, you will not need to have this 
> check and also do this weird thing below. Simply have the first check and 
> otherwise you can do just d->priorityFlowControl[userPriority] = enabled.

I think it is better to have this as a safety measure, and initialising so many 
values manually wold take 8*6=48 lines in the beginning of the file

> jgrulich wrote in dcbsetting.cpp:212
> Check just if userPriority is < 8.

that may lead to a malloc assertion error sometimes, and thus a crash which is 
confusing to understand

> jgrulich wrote in dcbsetting.cpp:393
> You can directly assign whole UintList. Same for the rest of options below.

I get a type casting error

> jgrulich wrote in dcbsetting.cpp:427
> Remove comments.

oops, i was debugging it

> jgrulich wrote in dcbsetting.cpp:428
> Shouldn't it save appFcoeMode() instead?

yeah, i uploaded some unfinished code

> jgrulich wrote in dcbsetting.cpp:464
> You can directly push the list there. Same for the lists below.

How?

> jgrulich wrote in dcbsetting.h:49
> You are missing Q_DECLARE_FLAGS()

I tried to do it in the same way as in ipv6 settings, so should i add 
Q_DECLARE_FLAGS() ?

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

To: pranavgade, jgrulich
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

Reply via email to