osmo-pcu[master]: Simplify TS alloc: improve readability

Max gerrit-no-reply at lists.osmocom.org
Sun Feb 4 19:46:55 UTC 2018


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 <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-HasComments: No


More information about the gerrit-log mailing list