PyKF5 bindings generator improvements out for review.

Shaheed Haque srhaque at theiet.org
Sun Jan 29 16:49:50 UTC 2017


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.

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?


On 29 January 2017 at 16:24, Stephen Kelly <steveire at gmail.com> wrote:

> Shaheed Haque wrote:
>
> > All the code is squashed into the single commit. Feedback welcome here or
> > via reviewboard or via github.
>
> Hi Shaheed,
>
> Having so many changes in a single commit is not really reviewable now or
> in
> the future. Commits should be minimal so that people in the future who
> examine the history can make sense of it. The commit message for that
> commit
> says
>
>  Re-introduce the missing databases.
>
> but in fact it appears to be a complete re-write in a single commit.
>
> Please extract meaningful changes into separate commits. For example, the
> change to
>
>  find-modules/Qt5Ruleset.py
>
> must correspond to only a part of the rest of the commit. Please extract
> that into a commit. Then consider any other changes in the big commit and
> extract small meaningful commits and repeat.
>
> When reviewing commits the reader should have only one context in mind. For
> example "ok, this commit is changing the API of rulesets" or "ok, this
> commit is introducing a noop so enable silencing warnings".
>
>  Usually that context for the commit should be in the commit message!
>
> Thanks,
>
> Steve.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-bindings/attachments/20170129/e41ed2db/attachment.html>


More information about the Kde-bindings mailing list