<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 6, 2019 at 5:46 PM Bryn M. Reeves <<a href="mailto:bmr@redhat.com">bmr@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Folks,<br>
<br>
I've experimentally turned on automatic LGTM reviews for sos pull<br>
requests.<br>
<br>
LGTM is a static code analysis suite for Python that produces a<br>
report on possible errors and language misuse. You can view the<br>
current report for sos master here:<br>
<br>
  <a href="https://lgtm.com/projects/g/sosreport/sos/" rel="noreferrer" target="_blank">https://lgtm.com/projects/g/sosreport/sos/</a><br>
<br>
There are roughly 50 currently, and on a quick scan most are an<br>
actionable bug (although some with very low/no impact). We'll<br>
be working through these over the next few weeks, although feel<br>
free to submit PRs for any outstanding items you notice.<br>
<br>
If the reviews seem useful we'll start to incorporate them in<br>
the routine sos review process.<br>
<br>
LMK if it LGTY!<br>
<br>
Regards,<br>
Bryn.<br>
</blockquote><div><br></div><div style="font-family:monospace,monospace" class="gmail_default">Nice and useful initiative, thanks for that!</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">I started to review the warnings, but I feel some of them are red herrings:</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">1) Both errors <a href="https://lgtm.com/projects/g/sosreport/sos/alerts/?mode=list&severity=error">https://lgtm.com/projects/g/sosreport/sos/alerts/?mode=list&severity=error</a> does not treat a r.e. in string<br></div><div style="font-family:monospace,monospace" class="gmail_default">like """</div><div style="font-family:monospace,monospace" class="gmail_default">     ^</div><div style="font-family:monospace,monospace" class="gmail_default">     ..</div><div style="font-family:monospace,monospace" class="gmail_default">     $<br></div><div style="font-family:monospace,monospace" class="gmail_default">     """</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">properly, as I think they treat the leading whitespace before ^ and before closing """ as valid chars (they arent)</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">2) for-else construction at <a href="https://lgtm.com/projects/g/sosreport/sos/snapshot/904808a29893d182621cf32cb9074b50f817b9a1/files/sos/plugins/__init__.py?sort=name&dir=ASC&mode=heatmap#L517">https://lgtm.com/projects/g/sosreport/sos/snapshot/904808a29893d182621cf32cb9074b50f817b9a1/files/sos/plugins/__init__.py?sort=name&dir=ASC&mode=heatmap#L517</a> is also valid - "return" _is_ kind of break :)</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default"></div><div style="font-family:monospace,monospace" class="gmail_default">3) dead code at <a href="https://github.com/sosreport/sos/blob/master/sos/sosreport.py#L126">https://github.com/sosreport/sos/blob/master/sos/sosreport.py#L126</a> revealed the fact no xml reports are generated at all :) (self.enabled = False all the time) - since this is the case for years, doesn't it make sense to remove the XmlReport completely? (that would get rid of 2 other warnings)<br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">For the 3) and many valid warnings, I raised #1559 now.</div><div style="font-family:monospace,monospace" class="gmail_default"><br></div><div style="font-family:monospace,monospace" class="gmail_default">Kind regards,</div><div style="font-family:monospace,monospace" class="gmail_default">Pavel<br></div><div style="font-family:monospace,monospace" class="gmail_default"><br></div></div></div></div></div></div></div></div></div>