[Notes] [Git][BuildStream/buildstream][master] CONTRIBUTING.rst: Some fixes in grammer and minor corrections

[Tristan Van Berkom <gitlab mg gitlab com>]

Title: GitLab

Tristan Van Berkom pushed to branch master at BuildStream / buildstream

Commits:

• 5587715c
  by Tristan Van Berkom at 2018-10-07T06:19:03Z

  CONTRIBUTING.rst: Some fixes in grammer and minor corrections
  

1 changed file:

• CONTRIBUTING.rst

Changes:

CONTRIBUTING.rst

---

...	...	@@ -90,13 +90,14 @@ BuildStream repository using the command line::
90	90
91	91	git push origin username/fix-that-bug:username/fix-that-bug
92	92
93		-GitLab will respond to this with a message and a link to allow
94		-you to create a new merge request.
	93	+GitLab will respond to this with a message and a link to allow you to create
	94	+a new merge request. You can also `create a merge request for an existing branch
	95	+<https://gitlab.com/BuildStream/buildstream/merge_requests/new>`_.
95	96
96		-You may open merge requests for the branches you create before you
97		-are ready to have them reviewed and considered for inclusion. As long as
98		-your merge request is not yet ready for review then the merge request
99		-title must be prefixed with the ``WIP:`` identifier.
	97	+You may open merge requests for the branches you create before you are ready
	98	+to have them reviewed and considered for inclusion if you like. Until your merge
	99	+request is ready for review, the merge request title must be prefixed with the
	100	+``WIP:`` identifier.
100	101
101	102
102	103	Organized commits
...	...	@@ -119,7 +120,7 @@ preferred in separate commits.
119	120
120	121	If a commit in your branch modifies behavior such that a test must also
121	122	be changed to match the new behavior, then the tests should be updated
122		-with the same commit, so that every commit in series passes it's own tests.
	123	+with the same commit, so that every commit passes it's own tests.
123	124
124	125
125	126	Commit messages
...	...	@@ -131,7 +132,7 @@ The summary line must start with what changed, followed by a colon and
131	132	a very brief description of the change.
132	133
133	134	If the commit fixes an issue, or is related to an issue; then the issue
134		-number should always be referenced in the commit message.
	135	+number must be referenced in the commit message.
135	136
136	137	**Example**::
137	138
...	...	@@ -165,7 +166,7 @@ separately.
165	166	o tests/artifactcache/expiry.py: Modified test expectations to
166	167	match the new behavior.
167	168
168		- Fixes #123
	169	+ This is a part of #123
169	170
170	171
171	172	Coding guidelines
...	...	@@ -183,12 +184,12 @@ in mind.
183	184
184	185	Approximate PEP-8 Style
185	186	~~~~~~~~~~~~~~~~~~~~~~~
186		-Python coding style for BuildStream is pep8, which is documented here: https://www.python.org/dev/peps/pep-0008/
	187	+Python coding style for BuildStream is approximately `pep8 <https://www.python.org/dev/peps/pep-0008/>`_.
187	188
188	189	We have a couple of minor exceptions to this standard, we dont want to compromise
189	190	code readability by being overly restrictive on line length for instance.
190	191
191		-The pep8 linter will run automatically when running the test suite.
	192	+The pep8 linter will run automatically when :ref:`running the test suite <contributing_testing>`.
192	193
193	194
194	195	.. _contributing_documenting_symbols:
...	...	@@ -234,9 +235,9 @@ comments and docstrings.
234	235	# Frobnicates this element with the specified source
235	236	#
236	237	# Args:
237		- # source (Source): The Source to frobnicate with
238		- # frobilicious (bool): Optionally specify that frobnication should be
239		- # performed fribiliciously
	238	+ # source (Source): The Source to frobnicate with
	239	+ # frobilicious (bool): Optionally specify that frobnication should be
	240	+ # performed fribiliciously
240	241	#
241	242	# Returns:
242	243	# (Element): The frobnicated version of this Element.
...	...	@@ -296,9 +297,6 @@ comments and docstrings.
296	297	# context (Context): The invocation Context
297	298	# count (int): The number to count
298	299	#
299		- # Returns:
300		- # (Foo): A newly created Foo object
301		- #
302	300	class Foo(Bar):
303	301	...
304	302
...	...	@@ -312,7 +310,7 @@ important to keep in mind the order in which symbols should appear
312	310	and keep this consistent.
313	311
314	312	Here is an example to illustrate the expected ordering of symbols
315		-on a python class in BuildStream::
	313	+on a Python class in BuildStream::
316	314
317	315	class Foo(Bar):
318	316
...	...	@@ -325,24 +323,24 @@ on a python class in BuildStream::
325	323
326	324	def __init__(self, name):
327	325
328		- super().__init__()
329		-
330		- # NOTE: In the instance initializer we declare any instance variables,
331		- # always declare the public instance variables (if any) before
332		- # the private ones.
333		- #
334		- # It is preferred to avoid any public instance variables, and
335		- # always expose an accessor method for it instead.
336		-
337		- #
338		- # Public instance variables
339		- #
340		- self.name = name # The name of this foo
341		-
342		- #
343		- # Private instance variables
344		- #
345		- self._count = 0 # The count of this foo
	326	+ super().__init__()
	327	+
	328	+ # NOTE: In the instance initializer we declare any instance variables,
	329	+ # always declare the public instance variables (if any) before
	330	+ # the private ones.
	331	+ #
	332	+ # It is preferred to avoid any public instance variables, and
	333	+ # always expose an accessor method for it instead.
	334	+
	335	+ #
	336	+ # Public instance variables
	337	+ #
	338	+ self.name = name # The name of this foo
	339	+
	340	+ #
	341	+ # Private instance variables
	342	+ #
	343	+ self._count = 0 # The count of this foo
346	344
347	345	################################################
348	346	# Abstract Methods #
...	...	@@ -371,12 +369,12 @@ on a python class in BuildStream::
371	369	def frob(self, count):
372	370
373	371	#
374		- # An abstract method in BuildStream is allowed to have
375		- # a default implementation.
376		- #
	372	+ # An abstract method in BuildStream is allowed to have
	373	+ # a default implementation.
	374	+ #
377	375	self._count = self._do_frobbing(count)
378	376
379		- return self._count
	377	+ return self._count
380	378
381	379	################################################
382	380	# Implementation of abstract methods #
...	...	@@ -388,9 +386,9 @@ on a python class in BuildStream::
388	386
389	387	def frobbish(self):
390	388	#
391		- # Implementation of the "frobbish" abstract method
392		- # defined by the parent Bar class.
393		- #
	389	+ # Implementation of the "frobbish" abstract method
	390	+ # defined by the parent Bar class.
	391	+ #
394	392	return True
395	393
396	394	################################################
...	...	@@ -443,10 +441,6 @@ on a python class in BuildStream::
443	441	# NOTE: Private methods are the ones which are internal
444	442	# implementation details of this class.
445	443	#
446		- # We can be absolutely sure that nobody is ever
447		- # going to call these functions from outside of
448		- # this class definition.
449		- #
450	444	# Even though these are private implementation
451	445	# details, they still MUST have API documenting
452	446	# comments on them.
...	...	@@ -469,17 +463,17 @@ on a python class in BuildStream::
469	463
470	464	Public and private symbols
471	465	~~~~~~~~~~~~~~~~~~~~~~~~~~
472		-BuildStream mostly follows the PEP-8 for defining public and private symbols
	466	+BuildStream mostly follows the PEP-8 for defining *public* and *private* symbols
473	467	for any given class, with some deviations. Please read the `section on inheritance
474	468	<https://www.python.org/dev/peps/pep-0008/#designing-for-inheritance>`_ for
475	469	reference on how the PEP-8 defines public and non-public.
476	470
477		-* A public symbol is any symbol which you expect to be used by clients
	471	+* A *public* symbol is any symbol which you expect to be used by clients
478	472	of your class or module within BuildStream.
479	473
480	474	Public symbols are written without any leading underscores.
481	475
482		-* A private symbol is any symbol which is entirely internal to your class
	476	+* A *private* symbol is any symbol which is entirely internal to your class
483	477	or module within BuildStream. These symbols cannot ever be accessed by
484	478	external clients or modules.
485	479
...	...	@@ -487,7 +481,7 @@ reference on how the PEP-8 defines public and non-public.
487	481
488	482	* When a class can have subclasses (for example, the ``Sandbox`` or ``Platform``
489	483	classes which have various implementations, or the ``Element`` and ``Source``
490		- classes which plugins derive from), then private sumbols should be denoted
	484	+ classes which plugins derive from), then private symbols should be denoted
491	485	by two leading underscores.
492	486
493	487	The double leading underscore naming convention invokes Python's name
...	...	@@ -514,7 +508,7 @@ such, we aim to keep the *"Public API Surface"* as small as possible at all
514	508	times, and never expose any internal details to plugins inadvertently.
515	509
516	510	One problem which arises from this is that we end up having symbols
517		-which are public according to the :ref:`rules discussed in the previous section
	511	+which are *public* according to the :ref:`rules discussed in the previous section
518	512	<contributing_public_and_private>`, but must be hidden away from the
519	513	*"Public API Surface"*. For example, BuildStream internal classes need
520	514	to invoke methods on the ``Element`` and ``Source`` classes, wheras these
...	...	@@ -541,7 +535,7 @@ were not discussed in :ref:`the class ordering section <contributing_class_order
541	535	these classes, the *"API Private"* symbols always come **before** the *"Local Private"*
542	536	symbols in the class declaration.
543	537
544		-Modules which are not a part of the *"Public API Surface"* have their python files
	538	+Modules which are not a part of the *"Public API Surface"* have their Python files
545	539	prefixed with a single underscore, and are not imported in BuildStream's the master
546	540	``__init__.py`` which is used by plugins.
547	541
...	...	@@ -590,8 +584,9 @@ is immutable for the object's life time (like an ``Element`` name for example),
590	584	is acceptable to save some typing by using a publicly accessible instance variable.
591	585
592	586	It is never acceptable to modify the value of an instance variable from outside
593		-of the declaring class. In other words, the class which exposes an instance variable
594		-is the only one in control of the value of this variable.
	587	+of the declaring class, even if the variable is *public*. In other words, the class
	588	+which exposes an instance variable is the only one in control of the value of this
	589	+variable.
595	590
596	591	* If an instance variable is public and must be modified; then it must be
597	592	modified using a :ref:`mutator <contributing_accessor_mutator>`
...	...	@@ -663,11 +658,11 @@ respectively, e.g.::
663	658	the ``@property`` decorator.
664	659
665	660	The decision to use explicitly defined functions instead of the
666		- ``@property`` feature is rather arbitrary, and there is not much
	661	+ ``@property`` decorator is rather arbitrary, there is not much
667	662	technical merit to preferring one technique over the other.
668	663	However as :ref:`discussed below <contributing_always_consistent>`,
669	664	it is of the utmost importance that we do not mix both techniques
670		- in the same code base.
	665	+ in the same codebase.
671	666
672	667
673	668	.. _contributing_abstract_methods:
...	...	@@ -696,7 +691,7 @@ is **illegal** to override any other method.
696	691	The key here is that in BuildStream, we consider it unacceptable
697	692	that a subclass overrides a method of it's parent class unless
698	693	the said parent class has explicitly given permission to subclasses
699		-to do so, and outlined the API contract for this. No surprises
	694	+to do so, and outlined the API contract for this purpose. No surprises
700	695	are allowed.
701	696
702	697
...	...	@@ -712,12 +707,13 @@ it is rarely necessary to handle a ``BstError``.
712	707	Raising Exceptions
713	708	''''''''''''''''''
714	709	When writing code in the BuildStream core, ensure that all system
715		-calls are wrapped in a ``try:`` block, and raise a descriptive ``BstError``
716		-of the appropriate class explaining what exactly failed.
	710	+calls and third party library calls are wrapped in a ``try:`` block,
	711	+and raise a descriptive ``BstError`` of the appropriate class explaining
	712	+what exactly failed.
717	713
718		-Always include the original system call error is formatted into
719		-your new exception, and that you use the python ``from`` semantic
720		-to retain the original call trace, example::
	714	+Ensure that the original system call error is formatted into your new
	715	+exception, and that you use the Python ``from`` semantic to retain the
	716	+original call trace, example::
721	717
722	718	try:
723	719	os.utime(self._refpath(ref))
...	...	@@ -728,8 +724,8 @@ to retain the original call trace, example::
728	724	Enhancing Exceptions
729	725	''''''''''''''''''''
730	726	Sometimes the ``BstError`` originates from a lower level component,
731		-and the source of the error did not have enough context to create
732		-a good and informative summary of the error for the user.
	727	+and the code segment which raised the exception did not have enough context
	728	+to create a complete, informative summary of the error for the user.
733	729
734	730	In these cases it is necessary to handle the error and raise a new
735	731	one, e.g.::
...	...	@@ -749,7 +745,7 @@ gracefully.
749	745
750	746	In these cases, do **not** raise any of the ``BstError`` class exceptions.
751	747
752		-Instead, use the python ``assert`` statement, e.g.::
	748	+Instead, use the ``assert`` statement, e.g.::
753	749
754	750	assert utils._is_main_process(), \
755	751	"Attempted to save workspace configuration from child process"
...	...	@@ -815,19 +811,19 @@ Python when compared to the way we do it in BuildStream, *do not do it*.
815	811	Consistency of how we do things in the codebase is more important
816	812	than the actual way in which things are done, always.
817	813
818		-Instead, if you like a certain python feature and think the BuildStream
819		-codebase, then propose your change on the `mailing list
	814	+Instead, if you like a certain Python feature and think the BuildStream
	815	+codebase should use it, then propose your change on the `mailing list
820	816	<https://mail.gnome.org/mailman/listinfo/buildstream-list>`_. Chances
821	817	are that we will reach agreement to use your preferred approach, and
822	818	in that case, it will be important to apply the change unilaterally
823	819	across the entire codebase, such that we continue to have a consistent
824		-code base.
	820	+codebase.
825	821
826	822
827	823	Avoid tail calling
828	824	~~~~~~~~~~~~~~~~~~
829	825	With the exception of tail calling with simple functions from
830		-the standard python library, such as splitting and joining lines
	826	+the standard Python library, such as splitting and joining lines
831	827	of text and encoding/decoding text; always avoid tail calling.
832	828
833	829	**Good**::
...	...	@@ -893,7 +889,7 @@ Vertical stacking of modules
893	889	For the sake of overall comprehensiveness of the BuildStream
894	890	architecture, it is important that we retain vertical stacking
895	891	order of the dependencies and knowledge of modules as much as
896		-possible.
	892	+possible, and avoid any cyclic relationships in modules.
897	893
898	894	For instance, the ``Source`` objects are owned by ``Element``
899	895	objects in the BuildStream data model, and as such the ``Element``
...	...	@@ -951,8 +947,8 @@ by adding new code paths or changing the design such that the
951	947	architecture continues to make sense.
952	948
953	949
954		-Refactor the code base as needed
955		-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	950	+Refactor the codebase as needed
	951	+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
956	952	Especially when implementing features, always move the BuildStream
957	953	codebase forward as a whole.
958	954
...	...	@@ -969,7 +965,7 @@ a ``Queue``. This means that you've proven that your feature can work,
969	965	and now it is time to start working on a patch for upstream.
970	966
971	967	Consider what the scenario is and why you are circumventing the design,
972		-and redesign the ``Scheduler`` and ``Queue`` objects to accommodate for
	968	+and then redesign the ``Scheduler`` and ``Queue`` objects to accommodate for
973	969	the new feature and condition under which you need to dispatch a ``Job``,
974	970	or how you can give the ``Queue`` implementation the additional context it
975	971	needs.
...	...	@@ -1101,7 +1097,7 @@ The BuildStream documentation style is as follows:
1101	1097	misuse of the API and explain it's consequences.
1102	1098
1103	1099	* Code blocks are defined using: ``.. code:: LANGUAGE`` blocks, followed by an empty
1104		- line and then indented (3 spaces) text. Note that the default language is `python`.
	1100	+ line and then indented (3 spaces) text. Note that the default language is ``python``.
1105	1101
1106	1102	* Cross references should be of the form ``:role:`target```.
1107	1103
...	...	@@ -1265,6 +1261,8 @@ regenerate them locally in order to build the docs.
1265	1261	command: build hello.bst
1266	1262
1267	1263
	1264	+.. _contributing_testing:
	1265	+
1268	1266	Testing
1269	1267	-------
1270	1268	BuildStream uses pytest for regression tests and testing out

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.