[BuildStream] Updated CONTRIBUTING.rst guide



Hi all,

As a result of the thread last month[0] where I outlined a hand full of
examples of code changes which should never have landed on master, and
described the negative consequences of those; we reached consensus that
we needed much stronger contributor guidelines.

Since we are becoming a rather large project, more than expected, it is
now inefficient to communicate all of the details of what we expect on
IRC or through review.

I have now pushed a new version of CONTRIBUTING.rst which should be
viewable very soon (after the pipeline for master completes) at:

    https://docs.buildstream.build/CONTRIBUTING.html

and

    https://buildstream.gitlab.io/buildstream/CONTRIBUTING.html


I want to strongly encourage the regular contributors to read through
the new CONTRIBUTING guide in detail, take the time to consume it now
and avoid headaches later. If there are things you strongly disagree
with, please reply to this email so we can discuss them openly.

The changes here mostly include:

  * More strict policy on submitting patches and formatting of commit
    messages

  * Complete, elaborate rewrite of coding guidelines, including a
    completely rewritten section regarding public/private symbols,
    which seems to be a constant source of confusion.

    This rewrite is a lot more rigid than before, placing emphasis
    more on doing things consistently across the codebase, and adding
    some more abstract architectural design concepts important for
    those who are developing more complex features.

  * Some re-ordering and restructuring of the file in general

Some interesting things came out of this, I'd like to draw attention to
the "Abstract methods" section of the coding guidelines, because this
might be a source of confusion for those who are more familiar with how
Python defines these.

I think it would be good to change our terminology on how we refer to
these methods which we create API contracts for subclasses to
implement, and possibly call them "Virtual methods" instead; implying
that if it is not a "Virtual method", it most certainly should never be
overridden unexpectedly by a malicious subclass (strongly worded I
know).

Some of the decisions on the minor and subtle points of how we do
things in BuildStream are rather arbitrary, and I would like to
emphasize here that the decision of how we do "a given thing" is much
much less important than ensuring that we *always* do a given thing in
the same way, across the entire codebase.

I would like to encourage those of you who have opinions on how we do
certain things, to reply and write the mailing list, proposing that we
make changes to the style - however know that the answer will always
be: If we're going to change it, we're going to change it for the
entire codebase, unilaterally, because consistency is more important.


Best Regards,
    -Tristan

[0]: https://mail.gnome.org/archives/buildstream-list/2018-September/msg00029.html



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]