On Wed, 27 Jun 2018 16:47:09 +0300 Pekka Paalanen <[email protected]> wrote:
> From: Pekka Paalanen <[email protected]> > > Half of the ideas came from Daniel but most of them are reworded, the > rest are my thoughts. > > Mention compiler warnings specifically, and be more explicit on what > kind of code or bugs or bug fixes are acceptable or not. Clarify commit > scope. > > Cc: Daniel Stone <[email protected]> > Signed-off-by: Pekka Paalanen <[email protected]> > --- > CONTRIBUTING.md | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > Hi, this is a bit more than already discussed. Questions below. > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md > index 70d0eca..51bef89 100644 > --- a/CONTRIBUTING.md > +++ b/CONTRIBUTING.md > @@ -223,11 +223,24 @@ include tests excercising the additions in the test > suite. > - The code fits the existing software architecture, e.g. no layering > violations. > > -- The code is correct and does not ignore corner-cases. > +- The code is correct and does not introduce new failures for existing users, > +does not add new corner-case bugs, and does not introduce new compiler > +warnings. > > -- In a patch series, every intermediate step produces correct code as well. > +- In a patch series, every intermediate step produces correct and > warning-free > +code as well. > > -- The code does what it says in the commit message and changes nothing else. > +- The patch does what it says in the commit message and changes nothing else. > + > +- The patch is a single logical change. If the commit message addresses > +multiple points, it is a hint that the commit might need splitting up. > + > +- A bug fix should target the underlying root cause instead of hiding > symptoms. > +If a complete fix is not practical, partial fixes are acceptable if they come > +with code comments and filed Gitlab issues for the remaining bugs. > + > +- The bug root cause rule applies to external software components as well, > e.g. > +do not work around kernel driver issues in userspace. This last item is written with Weston in mind. We definitely do not want to collect a pile of per-driver workarounds or features. However, sometimes a workaround has its place, drm_output_pick_crtc() works around bad possible_crtcs and possible_clones, for instance. https://gitlab.freedesktop.org/wayland/weston/blob/78a42116ae92f93a01539a785ce95cc478189608/libweston/compositor-drm.c#L4809 Is there a better wording to allow that? Or does this workaround have its place? Seeing that Ville Syrjälä went on a journey to actually fix the possible_*, I have filed an issue to remind us about it: https://gitlab.freedesktop.org/wayland/weston/issues/120 Thanks, pq
pgpYvYB3NLWHH.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
