Re: code review

The CSD patches were numerous and complex, too. These were merged WITHIN A SINGLE DAY. The contrast to MSC voice and HNBGW cnpool can hardly be more extreme. You are asking for a list of patches -- I'd rather not go into details there even more, but i found it hard to get through code review for the later half of https://gerrit.osmocom.org/q/cnpool+project:osmo-hnbgw And the "sensible time stamp" patches https://gerrit.osmocom.org/q/topic:log_timestamps Of course I agree with all of these points, and it is not like my responses to the CR were "go away you are stupid". I have very concise reasons for choices made, and it felt like these were brushed aside. So I explained again... Reasons for coding choices are usually complex. So if code review concentrates on inherently marginal items, and me saying so doesn't help, then we're stuck in a discussion of complex and precise reasons why a stupendously unimportant line of code was written that way. All of these discussions are absolutely valid in principle. But all of these can reach a point of losing touch with what is important. For example, if my patches *omitted* all logging, no CR would have complained. What made me write the mail is: a customer is waiting for my merge, and we're stuck discussing logging. At the same time I am not getting a chance to even read the pretty complex CSD patches before they were merged. To me it felt like a double standard being applied, certainly out of circumstance, not on purpose. Nevertheless I would welcome some agreement that this wasn't exactly optimal. ack yes, but the merge was too quick for even that. Yes, but now it is my work load: i have to modify the status quo and verify with testing etc (and apply code review), so in practice this is unlikely to happen. I don't get to drop a comment and continue my own task =) yes, i wish there were one single, sensible format. We had a lot of options before i started: print category-hex, print category, print level -- of course everyone wants to see both log level and category, and obviously the category *name* and not some hex number... I wanted other things changed because they way logs are structured greatly affects debugging cycles. For example, if source file and line are printed in the beginning, it heavily breaks up all "indenting" / lining up of log lines above and below. Another example is the "extended-timestamp" where it is extrememly annoying to find the numbers for hour, minute and second: on every line, i have to count digits from the right. These small invonveniences add up every minute, every hour, every day of debugging. The only chance to change it compatibly is to add options. I have another version of it that does away with the log timezone and the timestamp format tests, but since that is in my own free time, I'm not quick to finish it. Probably next time when I am reading logs the next time and find huge time wasted on the timestamp format, that's when I will feel that it's a good idea to put effort into getting a useful timestamp... For me the old formats are bloat; the only one that makes actual sense is one showing millis and having separators like 2018-01-16,01:44:34.681 I'm aware of that, and often thought whether we could change the default logging of osmo to a single accepted format, which has no omissions and is practical to use. ~N