Patch Set 7: Code-Review-1

> I don't rally get you removing the debugging code.

That's not debugging code in a sense that it's impossible to use it at runtime 
due to the load it introduce which interferes with actual PCU functionality. 
AFAIK that's the reason it was disabled in a first place.

> I think holger has raised a question why it was not helpful or what is your 
> concern herre, but there was - AFAIK - no response.

The response was that there're following reasons for removal: a) code is not 
actually used (permanently compile-time disabled) and b) code itself is hard to 
follow.

> are you confident that you are not removing useful log output without having 
> a functionally similarly complete replacement in your new code?

I'm confident that we're not loosing any log output that way because code in 
question is disabled at compile-time so it doesn't generate any log.

In general, I think it's much better to add unit tests for helper functions 
which are used in allocation and add logging there. That way we don't have to 
keep dead code. I'll double-check and update preceding patches, let's keep this 
one in gerrit till than as a reminder.

-- 
To view, visit https://gerrit.osmocom.org/4086
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I31600462e48d945bc8b7abf86a3718ac83e1dcbb
Gerrit-PatchSet: 7
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msur...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msur...@sysmocom.de>
Gerrit-HasComments: No

Reply via email to