<p style="white-space: pre-wrap; word-wrap: break-word;">(meh, the new gerrit UI keeps making me miss comments. I don't get it, some comments are plain visible, others are greyed out in a single line and look like jenkins cruft.)</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">1- I didn't say it was easy, yet is something really desirable.<br>3- It's not only about reviewing for merge. It's a problem about discussing each change.<br>It's a problem when reading the git log.<br>It's a problem when bisecting.<br>It's a problem when understanding why and how stuff was changed, and the implications it had.<br>It's a problem when reverting.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I still agree completely. We don't need to debate the above points at all.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Instead we need to discuss the practical tradeoff, which is quite debatable. At least for sponsored work, a large part of the question is: How much budget do you want to spend on it and what is the actual practical positive impact you expect from it?</p><p style="white-space: pre-wrap; word-wrap: break-word;">So let's say we do spend another week on separating this patch into three different ones. Then each of this infrastructure implemented on its own will need other (probably weird/ad hoc) inventions to make the intermediate code work. Instead of one large change that makes sense, you get three changes that each take a large step into weird no-mans-land of ad hoc solutions. I doubt that this would help during bisecting, likely some bugs appear in-between the patches to be fixed again right away, and reviewers / log readers need to understand ad-hoc shims just to see them being removed again right in the next patch. That's what I saw coming out of separating, so I stopped that. I really mean that you should try to separate it if you have the time, then it will become obvious how it is not trivial.</p><p style="white-space: pre-wrap; word-wrap: break-word;">This seems to me a discussion whether we want large changes at all. It is not possible to trivially change osmo-hlr to achieve a big change like this.</p><p style="white-space: pre-wrap; word-wrap: break-word;">BTW, we have used this code at 36c3 in december. OsmoHLR hasn't made any problems.</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">2- I agree with keeping ipa-name as a null-terminated string.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I started out thinking that we would use this struct for all kinds of identification, i.e. also as Global Title buffer, i.e. that we need to regard it as opaque blob of data. The code now seems to go a different direction, so I guess I don't know which way to take this. I don't want to pass it as an allocated string to avoid ownership. But keeping a separate length seems unnecessary... We did do a bunch of work previously to handle it as opaque blob, explicitly allowing both with nul and without nul; now I could use some help deciding what it should actually be...</p><p style="white-space: pre-wrap; word-wrap: break-word;"><br></p><p><a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16205">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-hlr/+/16205">change 16205</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-hlr/+/16205"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-hlr </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I3a8dff3d4a1cbe10d6ab08257a0138d6b2a082d9 </div>
<div style="display:none"> Gerrit-Change-Number: 16205 </div>
<div style="display:none"> Gerrit-PatchSet: 26 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 29 Jan 2020 11:44:16 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>