Re: PyKF5 bindings generator improvements out for review.

Shaheed Haque <[email protected]>
Newsgroups gmane.comp.kde.devel.bindings
Message-ID <CAHAc2jcQ8QVk_LvjMX+gkJ78PQu8Lv=TQdfMPN_r_ujJYdGv+Q@mail.gmail.com>
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 <[email protected]> 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.
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.