Helping in reviewing

Nick Kisialiou kisialiou at gmail.com
Tue Mar 26 12:50:46 PDT 2013


I'm not a Wayland developer but I suspect it wouldn't be wise to lump
Wayland (protocol) and Weston (compositor) together on this issue.

Wayland:
I'm not sure why it is a problem that Wayland patches take time to be
merged. Isn't it the whole point of any protocol to be as stable as
possible? If the protocol is constantly in some fluid experimental state
then toolkit developers have no incentive to port their code. Outside of
that, anyone who wants to do their experiments can easily do so in their
local sandbox -- the code is open source after all.

Weston:
IMHO, Weston patches don't need to go through the same rigorous review
process.

NK


On Tue, Mar 26, 2013 at 10:52 AM, Thiago Macieira <thiago.macieira at intel.com
> wrote:

> Recent circumstances have shown that patch reviewing in Wayland and Weston
> is
> becoming a bottleneck for development (at least, it is perceived to be).
> This
> email is intended to begin addressing that.
>
> TL;DR: if you want to help, the best thing you can do right now is review
> other people's contributions.
>
> Longer:
>
> Correct or not, the perception is that only Kristian has the authority to
> review and accept patches into Wayland and Weston. For that reason, it
> seems
> that no one else reviews patches, leaving them all to Kristian. In turn,
> that
> means Kristian has a lot of reviews to do, in addition to writing code.
> That
> does not scale.
>
> Everyone can help in reviewing, from the newest member of the community to
> the
> oldest. Reviewing is basically two separate operations:
>
> (1) technical (objective) review:
>  - is the code correct?
>  - does it compile? Does it introduce new bugs?
>  - does it fix the problem that it proposed to fix? Or does it successfully
>    implement the feature it proposed to implement?
>  - will it pose future problems, architecturally, security-wise, etc.?
>  - does it have unit tests? Is it documenting the API?
>  - is the code style correct?
>  - ...
>
> (2) subjective review:
>  - does this solution belong in Wayland or Weston?
>  - is it in scope for the project?
>  - is it the right solution for the problem?
>  - is the API easy to use? Not confusing?
>  - is the code readable, and properly commented?
>  - is this the right time for this? Is this the best use of resources?
>  - ...
>
> In both cases, the more experience you have in reviewing, the better the
> review will be and the easier it will be to review. However, for a new
> contributor, starting with the technical is easier, since it's objective.
> In
> time, you'll learn how to do the subjective review too.
>
> Also, don't be afraid of doing bad reviews. Other people will review the
> same
> contribution and review reviews. If a mistake is found, everyone learns.
> For
> the subjective review, there is really no right or wrong, but a consensus
> of
> the community. So speak up.
>
> Especially, speak up if the patch looks good. If you think it passes the
> review criteria, *say so*. Reply to the email saying "This looks good",
> "Ship
> it!" or "Awesome, thanks!" (BTW, praise goes a long way towards making
> people
> feel welcome).
>
> Kristian and other designated people with repository access will gladly
> accept
> those reviews and apply the contribution to the codebase. Of course, they
> reserve the right to re-review and point out if there were any problems.
> But
> just like above, if that happens, everyone learns.
>
> => Why you should do this?
>
> Well, if you're here, it's because you want Wayland and/or Weston to
> succeed.
> If you contribute a bit of your time and expertise, you're helping the
> project
> achieve that goal.
>
> If you have patches you have submitted yourself, by reviewing you free up
> the
> time from other reviewers, increasing the chances that your contribution
> will
> be dealt with sooner. Think also of this as tit-for-tat: if you want code
> reviewed, review code too.
>
> You'll also gain experience in reviewing. You'll become more familiar with
> the
> codebase. That might come in very handy if you're doing this as part of
> some
> company project.
>
> And you'll gain merit in the project. The more you contribute (in reviews
> or
> in code, but those go hand-in-hand), the more you'll be respected and the
> more
> your own opinions will be listened to. This is important when it comes to
> consensus-building and decision-making: Open Source Projects are not
> democracies and definitely not tyrannies. They are meritocracies, where
> those
> who have contributed the most get to make decisions.
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130326/59d71405/attachment.html>


More information about the wayland-devel mailing list