<p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235">View Change</a></p><p>6 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/ccid_common/ccid_slot_fsm.c">File ccid_common/ccid_slot_fsm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/ccid_common/ccid_slot_fsm.c@205">Patch Set #1, Line 205:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">/      if (cs->slot_nr == 0) {<br>//          cs->icc_present = true;<br>//          devname = "/dev/ttyUSB5";<br>// }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">that's what we have revision control for, so typically we don't want to merge commits that add commented-out parts of the original code.. Also, it will likely break the operation on the host side?  In that case a more generic solution must be found.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/cuart_driver_asf4_usart_async.c">File sysmoOCTSIM/cuart_driver_asf4_usart_async.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/cuart_driver_asf4_usart_async.c@167">Patch Set #1, Line 167:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">let's please use /* comments all over the code, thanks.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/gcc/Makefile">File sysmoOCTSIM/gcc/Makefile:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/gcc/Makefile@12">Patch Set #1, Line 12:</a> <code style="font-family:monospace,monospace">       -g3 -Wall -c -std=gnu99 $(CFLAGS_CPU)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">maybe a separate commit with rationale?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/libosmo_emb.c">File sysmoOCTSIM/libosmo_emb.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/libosmo_emb.c@163">Patch Set #1, Line 163:</a> <code style="font-family:monospace,monospace">      /* timer */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">might also be a separate commit?  or some kind of #define that you can enable at compile time to decide if logging shall be used or not?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/main.c">File sysmoOCTSIM/main.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/main.c@109">Patch Set #1, Line 109:</a> <code style="font-family:monospace,monospace">   </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">see other comment about not commenting out parts of the original code</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/usb_descriptors.c">File sysmoOCTSIM/usb_descriptors.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/usb_descriptors.c@a182">Patch Set #1, Line 182:</a> <code style="font-family:monospace,monospace">,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">maybe do the descriptor related changes as separate commit (or even multiple), stating the rationale for the respective change?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235">change 16235</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-ccid-firmware/+/16235"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-ccid-firmware </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia9d3b099181c24feabdb6a5fddb8db6c44efb72d </div>
<div style="display:none"> Gerrit-Change-Number: 16235 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 26 Nov 2019 23:18:31 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>