<blockquote>
<p>Tell us what needs improvement.</p>
</blockquote>
<p>The easy part</p>
<ul>
<li>Don't make white space changes</li>
<li>Keep   <code>/* comment */</code> style,  don't introduce    <code>//  comment</code>  style</li>
</ul>
<p>The</p>
<pre><code>-       struct asn1_cursor cursor;
+       struct asn1_cursor req, resp;
</code></pre>
<p>makes it hard to read what is actually added.</p>
<p>I can't see if the  unsqaushed commits made more clear what is added. It is gone in the update of this pull request.</p>
<p>I think the commit message would come better if the thrid line would start with<br>
(Thrid line:   First line is "commit summary", second line is empty )</p>
<pre><code>Implements parsing of all certID fields of incoming OCSP
responses as well as validation of the field values against the
respective request field values.
</code></pre>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/ipxe/ipxe/pull/90#issuecomment-466655654">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AArTVH8IVqUCbJ1A3xM9-ht6vl_-N1J4ks5vQU9IgaJpZM4a-EdD">mute the thread</a>.<img src="https://github.com/notifications/beacon/AArTVKKYb7HFyZaWF7a13PadDa95enG5ks5vQU9IgaJpZM4a-EdD.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/ipxe/ipxe","title":"ipxe/ipxe","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/ipxe/ipxe"}},"updates":{"snippets":[{"icon":"PERSON","message":"@stappersg in #90: \u003e Tell us what needs improvement.\r\n\r\nThe easy part\r\n* Don't make white space changes\r\n* Keep   `/* comment */` style,  don't introduce    `//  comment`  style\r\n\r\n\r\nThe\r\n```\r\n-       struct asn1_cursor cursor;\r\n+\tstruct asn1_cursor req, resp;\r\n```\r\nmakes it hard to read what is actually added.\r\n\r\nI can't see if the  unsqaushed commits made more clear what is added. It is gone in the update of this pull request.\r\n\r\nI think the commit message would come better if the thrid line would start with\r\n(Thrid line:   First line is \"commit summary\", second line is empty )\r\n```\r\nImplements parsing of all certID fields of incoming OCSP\r\nresponses as well as validation of the field values against the\r\nrespective request field values.\r\n```\r\n"}],"action":{"name":"View Pull Request","url":"https://github.com/ipxe/ipxe/pull/90#issuecomment-466655654"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/ipxe/ipxe/pull/90#issuecomment-466655654",
"url": "https://github.com/ipxe/ipxe/pull/90#issuecomment-466655654",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>