<div dir="ltr">Rewrite? It think its more a case of forking making things diverge (if you look, a lot of the changes are formatting, name changes and comments, though I accept that does make for a lot of noise). I have tried VERY hard to keep the good changes form both sides...and yes, there ARE a reasonable number of actual changes too.<br><br>Anyway, I was under the impression a squashed single commit was preferred, but I can look to pull things out again. I would prefer to work the reviews via github's Pull Requests, is that still OK?<br><div class="gmail_extra"><br><br><div class="gmail_quote">On 29 January 2017 at 16:24, Stephen Kelly <span dir="ltr"><<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">Shaheed Haque wrote:<br>
<br>
> All the code is squashed into the single commit. Feedback welcome here or<br>
> via reviewboard or via github.<br>
<br>
</span>Hi Shaheed,<br>
<br>
Having so many changes in a single commit is not really reviewable now or in<br>
the future. Commits should be minimal so that people in the future who<br>
examine the history can make sense of it. The commit message for that commit<br>
says<br>
<br>
Re-introduce the missing databases.<br>
<br>
but in fact it appears to be a complete re-write in a single commit.<br>
<br>
Please extract meaningful changes into separate commits. For example, the<br>
change to<br>
<br>
find-modules/Qt5Ruleset.py<br>
<br>
must correspond to only a part of the rest of the commit. Please extract<br>
that into a commit. Then consider any other changes in the big commit and<br>
extract small meaningful commits and repeat.<br>
<br>
When reviewing commits the reader should have only one context in mind. For<br>
example "ok, this commit is changing the API of rulesets" or "ok, this<br>
commit is introducing a noop so enable silencing warnings".<br>
<br>
Usually that context for the commit should be in the commit message!<br>
<br>
Thanks,<br>
<br>
Steve.<br>
</blockquote></div><br></div></div>