On Thu, 5 May 2016 22:45:38 +0200 Benoit Gschwind <gschw...@gnu-log.net> wrote:
> v3: > - rebase to master > - heavy patches spliting to help review as suggested by Pekka > - fix typo. > - rename functions and variables as suggested by Pekka > > Benoit Gschwind (17): > compositor-wayland: fix misc. typo > compositor-wayland: add versionning to config structure > compositor-wayland: rename wayland_output_init_from_config > compositor-wayland: refactor wayland_output_config_init > compositor-wayland: move configuration parsing to main.c > compositor-wayland: refactor load_wayland_backend > compositor-wayland: refactor load_wayland_backend > compositor-wayland: refactor load_wayland_backend > compositor-wayland: refactor wayland_backend_config_add_new_output > compositor-wayland: refactor wayland_backend_config_release > compositor-wayland: refactor wayland_backend_create > compositor-wayland: refactor wayland_backend_create > compositor-wayland: refactor backend_init > compositor-wayland: refactor backend_init > compositor-wayland: rename wayland_backend_config_release > compositor-wayland: rename wayland_output_config_init > compositor-wayland: rename wayland_backend_config_add_new_output > > src/compositor-wayland.c | 237 > +++++------------------------------------------ > src/compositor-wayland.h | 3 + > src/main.c | 223 +++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 246 insertions(+), 217 deletions(-) > Hi Benoit, thanks for the splitting. It may seem much, but it does make reviewing them very easy. The first thing that catches my eye is that several patches have identical summaries. That is a very poor practise, as the summary is supposed to be practically a unique name for the patch. Also, if there are three patches that all just "refactor load_wayland_backend", why are they not squashed into one? If each patch does something different, the summary should be different too. So, fix the messages, please. Several patches got the same comment as patch 4 that it is an ok change, but the commit message and summary needs improving. Patches that I did not explictly reply to but get the same comments are: 10 - 17. I also saw some repeated renames, like wayland_output_init_from_config -> wayland_output_config_init -> weston_wayland_output_config_init. The intermediate name has no benefit, so just rename once to the final name, please. Thanks, pq
pgpNdHI07PVr6.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
