<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body>
<br>
Just throwing this in, I agree with Neil very strongly here.&nbsp; Ive seen some patches recently with ~20 lines of change and 200 lines of whitespace.<br>
<br>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> lustre-devel &lt;lustre-devel-bounces@lists.lustre.org&gt; on behalf of NeilBrown &lt;neilb@suse.com&gt;<br>
<b>Sent:</b> Monday, December 17, 2018 6:09:57 PM<br>
<b>To:</b> James Simmons; Andreas Dilger; Oleg Drokin; Bobi Jam; Jinshan Xiong<br>
<b>Cc:</b> Lustre Development List<br>
<b>Subject:</b> Re: [lustre-devel] [PATCH 03/28] lustre: lov: merge lov_mds_md_v3 and lov_mds_md_v1 handling</font>
<div>&nbsp;</div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Mon, Dec 17 2018, James Simmons wrote:<br>
<br>
&gt; From: Bobi Jam &lt;bobijam@hotmail.com&gt;<br>
&gt;<br>
&gt; Several of the struct lsm_operations functions for both v1 and v3<br>
&gt; are nearly identical. Let merge them together.<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; Let's<br>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; <br>
&gt;&nbsp; <br>
&gt; -const struct lsm_operations lsm_v1_ops = {<br>
&gt; -&nbsp;&nbsp;&nbsp;&nbsp; .lsm_free&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; = lsm_free_plain,<br>
&gt; -&nbsp;&nbsp;&nbsp;&nbsp; .lsm_stripe_by_index&nbsp;&nbsp;&nbsp; = lsm_stripe_by_index_plain,<br>
&gt; -&nbsp;&nbsp;&nbsp;&nbsp; .lsm_stripe_by_offset&nbsp;&nbsp; = lsm_stripe_by_offset_plain,<br>
&gt; -&nbsp;&nbsp;&nbsp;&nbsp; .lsm_lmm_verify&nbsp; = lsm_lmm_verify_v1,<br>
&gt; -&nbsp;&nbsp;&nbsp;&nbsp; .lsm_unpackmd&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; = lsm_unpackmd_v1,<br>
&gt; &#43;const static struct lsm_operations lsm_v1_ops = {<br>
&gt; &#43;&nbsp;&nbsp;&nbsp;&nbsp; .lsm_stripe_by_index&nbsp;&nbsp;&nbsp; = lsm_stripe_by_index_plain,<br>
&gt; &#43;&nbsp;&nbsp;&nbsp;&nbsp; .lsm_stripe_by_offset&nbsp;&nbsp; = lsm_stripe_by_offset_plain,<br>
&gt; &#43;&nbsp;&nbsp;&nbsp;&nbsp; .lsm_lmm_verify&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; = lsm_lmm_verify_v1,<br>
&gt; &#43;&nbsp;&nbsp;&nbsp;&nbsp; .lsm_unpackmd&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; = lsm_unpackmd_v1,<br>
<br>
I *SO* wish you would stop combining white-spaces fixes with other<br>
changes in the same patch!!!<br>
The above hunk should just add the 'static' and remove the 'lsm_free'.<br>
The rest is just noise and makes it harder to review the patch.<br>
<br>
<br>
Thanks,<br>
NeilBrown<br>
</div>
</span></font></div>
</body>
</html>