[PATCH 1/3] contributor-guide: add missing links to mailing lists

[PATCH 1/3] contributor-guide: add missing links to mailing lists

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

And fix such a link too.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
---
 documentation/contributor-guide/submit-change.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/documentation/contributor-guide/submit-change.rst b/documentation/contributor-guide/submit-change.rst
index ba56e5986d..2555767102 100644
--- a/documentation/contributor-guide/submit-change.rst
+++ b/documentation/contributor-guide/submit-change.rst
@@ -52,7 +52,7 @@ varies by component:
 
 For changes to other layers hosted in the Yocto Project source
 repositories (i.e. ``yoctoproject.org``) and tools use the
-:yocto_lists:`Yocto Project </g/yocto/>` general mailing list.
+:yocto_lists:`yocto </g/yocto/>` general mailing list.
 
 .. note::
 
@@ -63,7 +63,8 @@ For additional recipes that do not fit into the core Metadata, you
 should determine which layer the recipe should go into and submit the
 change in the manner recommended by the documentation (e.g. the
 ``README`` file) supplied with the layer. If in doubt, please ask on the
-Yocto general mailing list or on the openembedded-devel mailing list.
+:yocto_lists:`yocto </g/yocto/>` general mailing list or on the
+:oe_lists:`openembedded-devel </g/openembedded-devel>` mailing list.
 
 You can also push a change upstream and request a maintainer to pull the
 change into the component's upstream repository. You do this by pushing
-- 
2.34.1

[PATCH 2/3] contributor-guide: add section about why we use mailing lists

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 .../contributor-guide/submit-change.rst       | 41 ++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/documentation/contributor-guide/submit-change.rst b/documentation/contributor-guide/submit-change.rst
index 2555767102..573491ecbc 100644
--- a/documentation/contributor-guide/submit-change.rst
+++ b/documentation/contributor-guide/submit-change.rst
@@ -8,10 +8,49 @@ Because the system is extremely configurable and flexible, we recognize
 that developers will want to extend, configure or optimize it for their
 specific uses.
 
+.. _ref-why-mailing-lists:
+
+Contributing through mailing lists --- Why not using web-based workflows?
+=========================================================================
+
+Both Yocto Project and OpenEmbedded have many key components that are
+maintained by patches being submitted on mailing lists. We appreciate this
+approach does look a little old fashioned when other workflows are available
+through web technology such as GitHub, GitLab and others. Since we are often
+asked this question, we’ve decided to document the reasons for using mailing
+lists.
+
+One significant factor is that we value peer review. When a change is proposed
+to many of the core pieces of the project, it helps to have many eyes of review
+go over them. Whilst there is ultimately one maintainer who needs to make the
+final call on accepting or rejecting a patch, the review is made by many eyes
+and the exact people reviewing it are likely unknown to the maintainer. It is
+often the surprise reviewer that catches the most interesting issues!
+
+This is in contrast to the "GitHub" style workflow where either just a
+maintainer makes that review, or review is specifically requested from
+nominated people. We believe there is significant value added to the codebase
+by this peer review and that moving away from mailing lists would be to the
+detriment of our code.
+
+We also need to acknowledge that many of our developers are used to this
+mailing list workflow and have worked with it for years, with tools and
+processes built around it. Changing away from this would result in a loss
+of key people from the project, which would again be to its detriment.
+
+The projects are acutely aware that potential new contributors find the
+mailing list approach off-putting and would prefer a point-and-click web GUI.
+Since we don’t believe that can work for us, the project is aiming to ensure
+`patchwork <https://patchwork.yoctoproject.org/>` is available to help track
+patch status and also looking at how tooling can provide more feedback to users
+about patch status. We are looking at tools such as ``patchtest`` to
+test user contributions before they hit the mailing lists and also at better
+documenting how to use such workflows since we recognise that whilst this was
+common knowledge a decade ago, it might not be as familiar now.
+
 Finding a Suitable Mailing List
 ===============================
 
-The Yocto Project uses a mailing list and a patch-based workflow that is
 similar to the Linux kernel but contains important differences. In
 general, there is a mailing list through which you can submit patches. You
 should send patches to the appropriate mailing list so that they can be
-- 
2.34.1

[PATCH 3/3] contributor-guide: add recipe style guide

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
---
 documentation/contributor-guide/index.rst     |   1 +
 .../contributor-guide/recipe-style-guide.rst  | 263 ++++++++++++++++++
 documentation/dev-manual/new-recipe.rst       |  54 +---
 3 files changed, 268 insertions(+), 50 deletions(-)
 create mode 100644 documentation/contributor-guide/recipe-style-guide.rst

diff --git a/documentation/contributor-guide/index.rst b/documentation/contributor-guide/index.rst
index d723854843..7a39f994e2 100644
--- a/documentation/contributor-guide/index.rst
+++ b/documentation/contributor-guide/index.rst
@@ -20,6 +20,7 @@ this.
 
    identify-component
    submit-defect
+   recipe-style-guide
    submit-change
 
 .. include:: /boilerplate.rst
diff --git a/documentation/contributor-guide/recipe-style-guide.rst b/documentation/contributor-guide/recipe-style-guide.rst
new file mode 100644
index 0000000000..5210062802
--- /dev/null
+++ b/documentation/contributor-guide/recipe-style-guide.rst
@@ -0,0 +1,263 @@
+.. SPDX-License-Identifier: CC-BY-SA-2.0-UK
+
+Recipe Style Guide
+******************
+
+Recipe Naming Conventions
+=========================
+
+In general, most recipes should follow the naming convention
+``recipes-category/package/packagename_version.bb``. Recipes for related
+projects may share the same package directory. ``packagename``, ``category``,
+and ``package`` may contain hyphens, but hyphens are not allowed in ``version``.
+
+If the recipe is tracking a Git revision that does not correspond to a released
+version of the software, ``version`` may be ``git`` (e.g. ``packagename_git.bb``)
+
+Version Policy
+==============
+
+Our versions follow the form ``<package epoch>:<package version>-<package revision>``
+or in BitBake variable terms ${:term:`PE`}:${:term:`PV`}-${:term:`PR`}. We
+generally follow the `Debian <https://www.debian.org/doc/debian-policy/ch-controlfields.html#version>`__
+version policy which defines these terms.
+
+In most cases the version :term:`PV` will be set automatically from the recipe
+file name. It is recommended to use released versions of software as these are
+revisions that upstream are expecting people to use.
+
+Package versions should always compare and sort correctly so that upgrades work
+as expected. With conventional versions such as ``1.4`` upgrading ``to 1.5``
+this happens naturally, but some versions don't sort. For example,
+``1.5 Release Candidate 2`` could be written as ``1.5rc2`` but this sorts after
+``1.5``, so upgrades from feeds won't happen correctly.
+
+Instead the tilde (``~``) operator can be used, which sorts before the empty
+string so ``1.5~rc2`` comes before ``1.5``. There is a historical syntax which
+may be found where :term:`PV` is set as a combination of the prior version
+``+`` the pre-release version, for example ``PV=1.4+1.5rc2``. This is a valid
+syntax but the tilde form is preferred.
+
+For version comparisons, the ``opkg-compare-versions`` program from
+``opkg-utils`` can be useful when attempting to determine how two version
+numbers compare to each other. Our definitive version comparison algorithm is
+the one within bitbake which aims to match those of the package managers and
+Debian policy closely.
+
+When a recipe references a git revision that does not correspond to a released
+version of software (e.g. is not a tagged version), the :term:`PV` variable
+should include the Git revision using the following to make the
+version clear::
+
+    PV = "<version>+git${SRCPV}"
+
+In this case, ``<version>`` should be the most recently released version of the
+software from the current source revision (``git describe`` can be useful for
+determining this). Whilst not recommended for published layers, this format is
+also useful when using :term:`AUTOREV` to set the recipe to increment source
+control revisions automatically, which can be useful during local development.
+
+Version Number Changes
+======================
+
+The :term:`PR` variable is used to indicate different revisions of a recipe
+that reference the same upstream source version. It can be used to force a
+new version of a package to be installed onto a device from a package feed.
+These once had to be set manually but in most cases these can now be set and
+incremented automatically by a PR Server connected with a package feed.
+
+When :term:`PV` increases, any existing :term:`PR` value can and should be
+removed.
+
+If :term:`PV` changes in such a way that it does not increase with respect to
+the previous value, you need to increase :term:`PE` to ensure package managers
+will upgrade it correctly. If unset you should set :term:`PE` to "1" since
+the default of empty is easily confused with "0" depending on the package
+manager. :term:`PE` can only have an integer value.
+
+Recipe formatting
+=================
+
+Variable Formatting
+-------------------
+
+-  Variable assignment should a space around each side of the operator, e.g.
+   ``FOO = "bar"``, not ``FOO="bar"``.
+
+-  Double quotes should be used on the right-hand side of the assignment,
+   e.g. ``FOO = "bar"`` not ``FOO = 'bar'``
+
+-  Spaces should be used for indenting variables, with 4 spaces per tab
+
+-  Long variables should be split over multiple lines when possible by using
+   the continuation character (``\``)
+
+-  When splitting a long variable over multiple lines, all continuation lines
+   should be indented (with spaces) to align with the start of the quote on the
+   first line::
+
+       FOO = "this line is \
+              long \
+              "
+
+   Instead of::
+
+       FOO = "this line is \
+       long \
+       "
+
+Python Function formatting
+--------------------------
+
+-  Spaces must be used for indenting Python code, with 4 spaces per tab
+
+Shell Function formatting
+-------------------------
+
+-  The formatting of shell functions should be consistent within layers.
+   Some use tabs, some use spaces.
+
+Recipe metadata
+===============
+
+Required Variables
+------------------
+
+The following variables should be included in all recipes:
+
+-  :term:`SUMMARY`: a one line description of the upstream project
+
+-  :term:`DESCRIPTION`: an extended description of the upstream project,
+   possibly with multiple lines. If no reasonable description can be written,
+   this may be omitted as it defaults to :term:`SUMMARY`.
+
+-  :term:`HOMEPAGE`: the URL to the upstream projects homepage.
+
+-  :term:`BUGTRACKER`: the URL upstream projects bug tracking website,
+   if applicable.
+
+Recipe Ordering
+---------------
+
+When a variable is defined in recipes and classes, variables should follow the
+general order when possible:
+
+-  :term:`SUMMARY`
+-  :term:`DESCRIPTION`
+-  :term:`AUTHOR`
+-  :term:`HOMEPAGE`
+-  :term:`BUGTRACKER`
+-  :term:`SECTION`
+-  :term:`LICENSE`
+-  :term:`LIC_FILES_CHKSUM`
+-  :term:`DEPENDS`
+-  :term:`PROVIDES`
+-  :term:`PV`
+-  :term:`SRC_URI`
+-  :term:`SRCREV`
+-  :term:`S`
+-  ``inherit ...``
+-  :term:`PACKAGECONFIG`
+-  Build class specific variables such as ``EXTRA_QMAKEVARS_POST`` and :term:`EXTRA_OECONF`
+-  Tasks such as :ref:`ref-tasks-configure`
+-  :term:`PACKAGE_ARCH`
+-  :term:`PACKAGES`
+-  :term:`FILES`
+-  :term:`RDEPENDS`
+-  :term:`RRECOMMENDS`
+-  :term:`RSUGGESTS`
+-  :term:`RPROVIDES`
+-  :term:`RCONFLICTS`
+-  :term:`BBCLASSEXTEND`
+
+There are some cases where ordering is important and these cases would override
+this default order. Examples include:
+
+-  :term:`PACKAGE_ARCH` needing to be set before ``inherit packagegroup``
+
+Tasks should be ordered based on the order they generally execute. For commonly
+used tasks this would be:
+
+-  :ref:`ref-tasks-fetch`
+-  :ref:`ref-tasks-unpack`
+-  :ref:`ref-tasks-patch`
+-  :ref:`ref-tasks-prepare_recipe_sysroot`
+-  :ref:`ref-tasks-configure`
+-  :ref:`ref-tasks-compile`
+-  :ref:`ref-tasks-install`
+-  :ref:`ref-tasks-populate_sysroot`
+-  :ref:`ref-tasks-package`
+
+Custom tasks should be sorted similarly.
+
+Package specific variables are typically grouped together, e.g.::
+
+    RDEPENDS:${PN} = “foo”
+    RDEPENDS:${PN}-libs = “bar”
+
+    RRECOMMENDS:${PN} = “one”
+    RRECOMMENDS:${PN}-libs = “two”
+
+Recipe License Fields
+---------------------
+
+Recipes need to define both the :term:`LICENSE` and
+:term:`LIC_FILES_CHKSUM` variables:
+
+-  :term:`LICENSE`: This variable specifies the license for the software.
+   If you do not know the license under which the software you are
+   building is distributed, you should go to the source code and look
+   for that information. Typical files containing this information
+   include ``COPYING``, :term:`LICENSE`, and ``README`` files. You could
+   also find the information near the top of a source file. For example,
+   given a piece of software licensed under the GNU General Public
+   License version 2, you would set :term:`LICENSE` as follows::
+
+      LICENSE = "GPL-2.0-only"
+
+   The licenses you specify within :term:`LICENSE` can have any name as long
+   as you do not use spaces, since spaces are used as separators between
+   license names. For standard licenses, use the names of the files in
+   ``meta/files/common-licenses/`` or the :term:`SPDXLICENSEMAP` flag names
+   defined in ``meta/conf/licenses.conf``.
+
+-  :term:`LIC_FILES_CHKSUM`: The OpenEmbedded build system uses this
+   variable to make sure the license text has not changed. If it has,
+   the build produces an error and it affords you the chance to figure
+   it out and correct the problem.
+
+   You need to specify all applicable licensing files for the software.
+   At the end of the configuration step, the build process will compare
+   the checksums of the files to be sure the text has not changed. Any
+   differences result in an error with the message containing the
+   current checksum. For more explanation and examples of how to set the
+   :term:`LIC_FILES_CHKSUM` variable, see the
+   ":ref:`dev-manual/licenses:tracking license changes`" section.
+
+   To determine the correct checksum string, you can list the
+   appropriate files in the :term:`LIC_FILES_CHKSUM` variable with incorrect
+   md5 strings, attempt to build the software, and then note the
+   resulting error messages that will report the correct md5 strings.
+   See the ":ref:`dev-manual/new-recipe:fetching code`" section for
+   additional information.
+
+   Here is an example that assumes the software has a ``COPYING`` file::
+
+      LIC_FILES_CHKSUM = "file://COPYING;md5=xxx"
+
+   When you try to build the
+   software, the build system will produce an error and give you the
+   correct string that you can substitute into the recipe file for a
+   subsequent build.
+
+Tips and Guidelines for Writing Recipes
+---------------------------------------
+
+-  Use :term:`BBCLASSEXTEND` instead of creating separate recipes such as ``-native``
+   and ``-nativesdk`` ones, whenever possible. This avoids having to maintain multiple
+   recipe files at the same time.
+
+-  Avoid manually mangling ``pkg-config`` ``.pc`` files.
+   Recipes using ``pkg-config`` should use variables to ensure they are correctly
+   relocatable and not need manual path correction in the recipe.
+
diff --git a/documentation/dev-manual/new-recipe.rst b/documentation/dev-manual/new-recipe.rst
index af390773a9..cb9533ff5d 100644
--- a/documentation/dev-manual/new-recipe.rst
+++ b/documentation/dev-manual/new-recipe.rst
@@ -432,56 +432,10 @@ named the same as the base name of the recipe
 Licensing
 =========
 
-Your recipe needs to have both the
-:term:`LICENSE` and
-:term:`LIC_FILES_CHKSUM`
-variables:
-
--  :term:`LICENSE`: This variable specifies the license for the software.
-   If you do not know the license under which the software you are
-   building is distributed, you should go to the source code and look
-   for that information. Typical files containing this information
-   include ``COPYING``, :term:`LICENSE`, and ``README`` files. You could
-   also find the information near the top of a source file. For example,
-   given a piece of software licensed under the GNU General Public
-   License version 2, you would set :term:`LICENSE` as follows::
-
-      LICENSE = "GPL-2.0-only"
-
-   The licenses you specify within :term:`LICENSE` can have any name as long
-   as you do not use spaces, since spaces are used as separators between
-   license names. For standard licenses, use the names of the files in
-   ``meta/files/common-licenses/`` or the :term:`SPDXLICENSEMAP` flag names
-   defined in ``meta/conf/licenses.conf``.
-
--  :term:`LIC_FILES_CHKSUM`: The OpenEmbedded build system uses this
-   variable to make sure the license text has not changed. If it has,
-   the build produces an error and it affords you the chance to figure
-   it out and correct the problem.
-
-   You need to specify all applicable licensing files for the software.
-   At the end of the configuration step, the build process will compare
-   the checksums of the files to be sure the text has not changed. Any
-   differences result in an error with the message containing the
-   current checksum. For more explanation and examples of how to set the
-   :term:`LIC_FILES_CHKSUM` variable, see the
-   ":ref:`dev-manual/licenses:tracking license changes`" section.
-
-   To determine the correct checksum string, you can list the
-   appropriate files in the :term:`LIC_FILES_CHKSUM` variable with incorrect
-   md5 strings, attempt to build the software, and then note the
-   resulting error messages that will report the correct md5 strings.
-   See the ":ref:`dev-manual/new-recipe:fetching code`" section for
-   additional information.
-
-   Here is an example that assumes the software has a ``COPYING`` file::
-
-      LIC_FILES_CHKSUM = "file://COPYING;md5=xxx"
-
-   When you try to build the
-   software, the build system will produce an error and give you the
-   correct string that you can substitute into the recipe file for a
-   subsequent build.
+Your recipe needs to define variables related to the license
+under whith the software is distributed. See the
+:ref:`contributor-guide/recipe-style-guide:recipe license fields`
+section in the Contributor Guide for details.
 
 Dependencies
 ============
-- 
2.34.1

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Frédéric Martinsons]

[-- Attachment #1.1: Type: text/plain, Size: 6461 bytes --]

On Wed, 9 Aug 2023 at 16:25, Michael Opdenacker via lists.yoctoproject.org
<michael.opdenacker=bootlin.com@lists.yoctoproject.org> wrote:

> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  .../contributor-guide/submit-change.rst       | 41 ++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/documentation/contributor-guide/submit-change.rst
> b/documentation/contributor-guide/submit-change.rst
> index 2555767102..573491ecbc 100644
> --- a/documentation/contributor-guide/submit-change.rst
> +++ b/documentation/contributor-guide/submit-change.rst
> @@ -8,10 +8,49 @@ Because the system is extremely configurable and
> flexible, we recognize
>  that developers will want to extend, configure or optimize it for their
>  specific uses.
>
> +.. _ref-why-mailing-lists:
> +
> +Contributing through mailing lists --- Why not using web-based workflows?
> +=========================================================================
> +
> +Both Yocto Project and OpenEmbedded have many key components that are
> +maintained by patches being submitted on mailing lists. We appreciate this
> +approach does look a little old fashioned when other workflows are
> available
> +through web technology such as GitHub, GitLab and others. Since we are
> often
> +asked this question, we’ve decided to document the reasons for using
> mailing
> +lists.


Great to read those lines, I was one of those who asked this question ;)


>
>
+One significant factor is that we value peer review. When a change is
> proposed
> +to many of the core pieces of the project, it helps to have many eyes of
> review
> +go over them. Whilst there is ultimately one maintainer who needs to make
> the
> +final call on accepting or rejecting a patch, the review is made by many
> eyes
> +and the exact people reviewing it are likely unknown to the maintainer.
> It is
> +often the surprise reviewer that catches the most interesting issues!
> +
> +This is in contrast to the "GitHub" style workflow where either just a
> +maintainer makes that review, or review is specifically requested from
> +nominated people. We believe there is significant value added to the
> codebase
> +by this peer review and that moving away from mailing lists would be to
> the
> +detriment of our code.
>

For notifications, I don't see why the web flow would not allow to send
notifications
for changes to a mailing list (I think it can also be made per user profile
to receive
notifications for a whole project, an issue, a particular project ... etc)


> +We also need to acknowledge that many of our developers are used to this
> +mailing list workflow and have worked with it for years, with tools and
> +processes built around it. Changing away from this would result in a loss
> +of key people from the project, which would again be to its detriment.
>

I'd like to know some of these flow that are plugged on mail, can we insert
some
examples in the documentation ?


> +The projects are acutely aware that potential new contributors find the
> +mailing list approach off-putting and would prefer a point-and-click web
> GUI.
>

I find the term "point-and-click" a little patronizing,
github/gitlab/others web tools
are not just point-and-click machine but highly customizable tool aims at
smoothen
the developer work (in term of tracking, viewing diff, forking) and to
provide
enhanced user experience.


> +Since we don’t believe that can work for us, the project is aiming to
> ensure
> +`patchwork <https://patchwork.yoctoproject.org/>` is available to help
> track
> +patch status and also looking at how tooling can provide more feedback to
> users
> +about patch status. We are looking at tools such as ``patchtest`` to
> +test user contributions before they hit the mailing lists and also at
> better
> +documenting how to use such workflows since we recognise that whilst this
> was
> +common knowledge a decade ago, it might not be as familiar now.
>

I didn't know patchtest, I'll look at it. I think a big improvement (at
least for me) in the review
process would be to some kind of colorized webview of a patch towards the
target branch.
Sorry, but , unless I miss some cool feature of my gmail client, I find
that view (screenshot join)

far more readable that raw diff by mail, e.g:

diff --git a/meta/classes-recipe/cargo.bbclass
b/meta/classes-recipe/cargo.bbclass
index 3ef0bbbb44..8c0b92df8d 100644
--- a/meta/classes-recipe/cargo.bbclass
+++ b/meta/classes-recipe/cargo.bbclass
@@ -39,7 +39,12 @@ MANIFEST_PATH ??= "${S}/${CARGO_SRC_DIR}/Cargo.toml"

 RUSTFLAGS ??= ""
 BUILD_MODE = "${@['--release', ''][d.getVar('DEBUG_BUILD') == '1']}"
-CARGO_BUILD_FLAGS = "-v --offline --target ${RUST_HOST_SYS} ${BUILD_MODE}
--manifest-path=${MANIFEST_PATH}"
+# --frozen flag will prevent network access (which is required since only
+# the do_fetch step is authorized to access network)
+# and will require an up to date Cargo.lock file.
+# This force the package being built to already ship a Cargo.lock, in the
end
+# this is what we want, at least, for reproducibility of the build.
+CARGO_BUILD_FLAGS = "-v --frozen --target ${RUST_HOST_SYS} ${BUILD_MODE}
--manifest-path=${MANIFEST_PATH}"

 # This is based on the content of CARGO_BUILD_FLAGS and generally will
need to
 # change if CARGO_BUILD_FLAGS changes.
-- 
2.34.1



> +
>  Finding a Suitable Mailing List
>  ===============================
>
> -The Yocto Project uses a mailing list and a patch-based workflow that is
>  similar to the Linux kernel but contains important differences. In
>  general, there is a mailing list through which you can submit patches. You
>  should send patches to the appropriate mailing list so that they can be
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#4121):
> https://lists.yoctoproject.org/g/docs/message/4121
> Mute This Topic: https://lists.yoctoproject.org/mt/100643892/6213388
> Group Owner: docs+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/docs/unsub [
> frederic.martinsons@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

[-- Attachment #1.2: Type: text/html, Size: 8889 bytes --]

[-- Attachment #2: Screenshot from 2023-08-09 17-56-57.png --]
[-- Type: image/png, Size: 239132 bytes --]

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Richard Purdie]

On Wed, 2023-08-09 at 17:58 +0200, Frédéric Martinsons wrote:
> On Wed, 9 Aug 2023 at 16:25, Michael Opdenacker via
> lists.yoctoproject.org
> <michael.opdenacker=bootlin.com@lists.yoctoproject.org> wrote:
> 
> > +One significant factor is that we value peer review. When a change
> > is proposed
> > +to many of the core pieces of the project, it helps to have many
> > eyes of review
> > +go over them. Whilst there is ultimately one maintainer who needs
> > to make the
> > +final call on accepting or rejecting a patch, the review is made
> > by many eyes
> > +and the exact people reviewing it are likely unknown to the
> > maintainer. It is
> > +often the surprise reviewer that catches the most interesting
> > issues!
> > +
> > +This is in contrast to the "GitHub" style workflow where either
> > just a
> > +maintainer makes that review, or review is specifically requested
> > from
> > +nominated people. We believe there is significant value added to
> > the codebase
> > +by this peer review and that moving away from mailing lists would
> > be to the
> > +detriment of our code.
> > 
> 
> 
> For notifications, I don't see why the web flow would not allow to
> send notifications for changes to a mailing list (I think it can also
> be made per user profile to receive notifications for a whole
> project, an issue, a particular project ... etc)

You can make a web interface send notices to an email list, sure, that
isn't hard. Someone then replies giving feedback on the mailing list to
the patch. What happens to that feedback from the web point of view?
This "next step" is where the challenge is.

Patchwork is an attempt to make that match up and it basically shows
how hard this is to get right and pair the worlds up like that.
>  
> > +We also need to acknowledge that many of our developers are used
> > to this
> > +mailing list workflow and have worked with it for years, with
> > tools and
> > +processes built around it. Changing away from this would result in
> > a loss
> > +of key people from the project, which would again be to its
> > detriment.
> 
> I'd like to know some of these flow that are plugged on mail, can we
> insert some examples in the documentation ?

You mean document how mailing list patch work flow works?

> > +The projects are acutely aware that potential new contributors
> > find the
> > +mailing list approach off-putting and would prefer a point-and-
> > click web GUI.
> > 
> 
> 
> I find the term "point-and-click" a little patronizing,
> github/gitlab/others web tools
> are not just point-and-click machine but highly customizable tool
> aims at smoothen the developer work (in term of tracking, viewing
> diff, forking) and to provide enhanced user experience.

I agree we should tweak the wording.

>  
> > +Since we don’t believe that can work for us, the project is aiming
> > to ensure
> > +`patchwork <https://patchwork.yoctoproject.org/>` is available to
> > help track
> > +patch status and also looking at how tooling can provide more
> > feedback to users
> > +about patch status. We are looking at tools such as ``patchtest``
> > to
> > +test user contributions before they hit the mailing lists and also
> > at better
> > +documenting how to use such workflows since we recognise that
> > whilst this was
> > +common knowledge a decade ago, it might not be as familiar now.
> > 
> 
> 
> I didn't know patchtest, I'll look at it. I think a big improvement
> (at least for me) in the review
> process would be to some kind of colorized webview of a patch towards
> the target branch.

If you make a pull request from the contrib repos, you can see
colorized diffs using the web interfaces. You can also see that locally
with git itself or tools like gitk too. I do use both the cgit
interface and gitk commands at different times.

> Sorry, but , unless I miss some cool feature of my gmail client, I
> find that view (screenshot join) far more readable that raw diff by
> mail, e.g:

It does depend upon what you're used to. Personally, I can hand edit or
even hand write diff files :/. I'm not saying that is a good idea or a
good thing but it is what it is.

Cheers,

Richard

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Frédéric Martinsons]

[-- Attachment #1: Type: text/plain, Size: 5426 bytes --]

Le mer. 9 août 2023, 19:57, Richard Purdie <
richard.purdie@linuxfoundation.org> a écrit :

> On Wed, 2023-08-09 at 17:58 +0200, Frédéric Martinsons wrote:
> > On Wed, 9 Aug 2023 at 16:25, Michael Opdenacker via
> > lists.yoctoproject.org
> > <michael.opdenacker=bootlin.com@lists.yoctoproject.org> wrote:
> >
> > > +One significant factor is that we value peer review. When a change
> > > is proposed
> > > +to many of the core pieces of the project, it helps to have many
> > > eyes of review
> > > +go over them. Whilst there is ultimately one maintainer who needs
> > > to make the
> > > +final call on accepting or rejecting a patch, the review is made
> > > by many eyes
> > > +and the exact people reviewing it are likely unknown to the
> > > maintainer. It is
> > > +often the surprise reviewer that catches the most interesting
> > > issues!
> > > +
> > > +This is in contrast to the "GitHub" style workflow where either
> > > just a
> > > +maintainer makes that review, or review is specifically requested
> > > from
> > > +nominated people. We believe there is significant value added to
> > > the codebase
> > > +by this peer review and that moving away from mailing lists would
> > > be to the
> > > +detriment of our code.
> > >
> >
> >
> > For notifications, I don't see why the web flow would not allow to
> > send notifications for changes to a mailing list (I think it can also
> > be made per user profile to receive notifications for a whole
> > project, an issue, a particular project ... etc)
>
> You can make a web interface send notices to an email list, sure, that
> isn't hard. Someone then replies giving feedback on the mailing list to
> the patch. What happens to that feedback from the web point of view?
> This "next step" is where the challenge is.
>

Yes I understand that.


> Patchwork is an attempt to make that match up and it basically shows
> how hard this is to get right and pair the worlds up like that.
> >
> > > +We also need to acknowledge that many of our developers are used
> > > to this
> > > +mailing list workflow and have worked with it for years, with
> > > tools and
> > > +processes built around it. Changing away from this would result in
> > > a loss
> > > +of key people from the project, which would again be to its
> > > detriment.
> >
> > I'd like to know some of these flow that are plugged on mail, can we
> > insert some examples in the documentation ?
>
> You mean document how mailing list patch work flow works?
>

No, I do know (or I think I know) how the patch flow works. I talked about
the text that says "with tools [...] built around it", I wouls like to know
what are these tools.


> > > +The projects are acutely aware that potential new contributors
> > > find the
> > > +mailing list approach off-putting and would prefer a point-and-
> > > click web GUI.
> > >
> >
> >
> > I find the term "point-and-click" a little patronizing,
> > github/gitlab/others web tools
> > are not just point-and-click machine but highly customizable tool
> > aims at smoothen the developer work (in term of tracking, viewing
> > diff, forking) and to provide enhanced user experience.
>
> I agree we should tweak the wording.
>
> >
> > > +Since we don’t believe that can work for us, the project is aiming
> > > to ensure
> > > +`patchwork <https://patchwork.yoctoproject.org/>` is available to
> > > help track
> > > +patch status and also looking at how tooling can provide more
> > > feedback to users
> > > +about patch status. We are looking at tools such as ``patchtest``
> > > to
> > > +test user contributions before they hit the mailing lists and also
> > > at better
> > > +documenting how to use such workflows since we recognise that
> > > whilst this was
> > > +common knowledge a decade ago, it might not be as familiar now.
> > >
> >
> >
> > I didn't know patchtest, I'll look at it. I think a big improvement
> > (at least for me) in the review
> > process would be to some kind of colorized webview of a patch towards
> > the target branch.
>
> If you make a pull request from the contrib repos, you can see
> colorized diffs using the web interfaces. You can also see that locally
> with git itself or tools like gitk too. I do use both the cgit
> interface and gitk commands at different times.
>

Yes of course, I can see my own patches the way I want. But what about
contributions from other that I'd like to read more carefully? I can pick
these patches and apply in my local checkout and see them "graphically" but
it is an additional step that may be skipped if we directly have them
somewhere. I agreed that patchwork have a better look interfaces than mail,
but as you said earlier, making the second step (i.e commenting a line,
suggesting changes... Etc) is not made in such interface.

>
> > Sorry, but , unless I miss some cool feature of my gmail client, I
> > find that view (screenshot join) far more readable that raw diff by
> > mail, e.g:
>
> It does depend upon what you're used to. Personally, I can hand edit or
> even hand write diff files :/. I'm not saying that is a good idea or a
> good thing but it is what it is.
>

Yes, I'm aware that it is a matter of taste and habits. I only want to gave
my point of view. Thank you very much for your answers.


> Cheers,
>
> Richard
>
>

[-- Attachment #2: Type: text/html, Size: 7632 bytes --]

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Daniel Ammann]

On 8/9/23 16:25, Michael Opdenacker via lists.yoctoproject.org wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   .../contributor-guide/submit-change.rst       | 41 ++++++++++++++++++-
>   1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/documentation/contributor-guide/submit-change.rst b/documentation/contributor-guide/submit-change.rst
> index 2555767102..573491ecbc 100644
> --- a/documentation/contributor-guide/submit-change.rst
> +++ b/documentation/contributor-guide/submit-change.rst
> @@ -8,10 +8,49 @@ Because the system is extremely configurable and flexible, we recognize
>   that developers will want to extend, configure or optimize it for their
>   specific uses.
>   
> +.. _ref-why-mailing-lists:
> +
> +Contributing through mailing lists --- Why not using web-based workflows?
> +=========================================================================
> +
> +Both Yocto Project and OpenEmbedded have many key components that are
> +maintained by patches being submitted on mailing lists. We appreciate this
> +approach does look a little old fashioned when other workflows are available
> +through web technology such as GitHub, GitLab and others. Since we are often
> +asked this question, we’ve decided to document the reasons for using mailing
> +lists.
> +
> +One significant factor is that we value peer review. When a change is proposed
> +to many of the core pieces of the project, it helps to have many eyes of review
> +go over them. Whilst there is ultimately one maintainer who needs to make the
> +final call on accepting or rejecting a patch, the review is made by many eyes
> +and the exact people reviewing it are likely unknown to the maintainer. It is
> +often the surprise reviewer that catches the most interesting issues!
> +
> +This is in contrast to the "GitHub" style workflow where either just a
> +maintainer makes that review, or review is specifically requested from
> +nominated people. We believe there is significant value added to the codebase
> +by this peer review and that moving away from mailing lists would be to the
> +detriment of our code.
> +
> +We also need to acknowledge that many of our developers are used to this
> +mailing list workflow and have worked with it for years, with tools and
> +processes built around it. Changing away from this would result in a loss
> +of key people from the project, which would again be to its detriment.
> +
> +The projects are acutely aware that potential new contributors find the
> +mailing list approach off-putting and would prefer a point-and-click web GUI.
> +Since we don’t believe that can work for us, the project is aiming to ensure
> +`patchwork <https://patchwork.yoctoproject.org/>` is available to help track
> +patch status and also looking at how tooling can provide more feedback to users
> +about patch status. We are looking at tools such as ``patchtest`` to
> +test user contributions before they hit the mailing lists and also at better
> +documenting how to use such workflows since we recognise that whilst this was
> +common knowledge a decade ago, it might not be as familiar now.
> +
>   Finding a Suitable Mailing List
>   ===============================
>   
> -The Yocto Project uses a mailing list and a patch-based workflow that is

Oops

>   similar to the Linux kernel but contains important differences. In
>   general, there is a mailing list through which you can submit patches. You
>   should send patches to the appropriate mailing list so that they can be
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#4121): https://lists.yoctoproject.org/g/docs/message/4121
> Mute This Topic: https://lists.yoctoproject.org/mt/100643892/3616718
> Group Owner: docs+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/docs/unsub [daniel.ammann@bytesatwork.ch]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Michael Opdenacker]

HI Daniel,

On 10.08.23 at 09:42, Daniel Ammann wrote:
> On 8/9/23 16:25, Michael Opdenacker via lists.yoctoproject.org wrote:
>> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>>
>> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> ---
>>   .../contributor-guide/submit-change.rst       | 41 ++++++++++++++++++-
>>   1 file changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/documentation/contributor-guide/submit-change.rst 
>> b/documentation/contributor-guide/submit-change.rst
>> index 2555767102..573491ecbc 100644
>> --- a/documentation/contributor-guide/submit-change.rst
>> +++ b/documentation/contributor-guide/submit-change.rst
>> @@ -8,10 +8,49 @@ Because the system is extremely configurable and 
>> flexible, we recognize
>>   that developers will want to extend, configure or optimize it for 
>> their
>>   specific uses.
>>   +.. _ref-why-mailing-lists:
>> +
>> +Contributing through mailing lists --- Why not using web-based 
>> workflows?
>> +========================================================================= 
>>
>> +
>> +Both Yocto Project and OpenEmbedded have many key components that are
>> +maintained by patches being submitted on mailing lists. We 
>> appreciate this
>> +approach does look a little old fashioned when other workflows are 
>> available
>> +through web technology such as GitHub, GitLab and others. Since we 
>> are often
>> +asked this question, we’ve decided to document the reasons for using 
>> mailing
>> +lists.
>> +
>> +One significant factor is that we value peer review. When a change 
>> is proposed
>> +to many of the core pieces of the project, it helps to have many 
>> eyes of review
>> +go over them. Whilst there is ultimately one maintainer who needs to 
>> make the
>> +final call on accepting or rejecting a patch, the review is made by 
>> many eyes
>> +and the exact people reviewing it are likely unknown to the 
>> maintainer. It is
>> +often the surprise reviewer that catches the most interesting issues!
>> +
>> +This is in contrast to the "GitHub" style workflow where either just a
>> +maintainer makes that review, or review is specifically requested from
>> +nominated people. We believe there is significant value added to the 
>> codebase
>> +by this peer review and that moving away from mailing lists would be 
>> to the
>> +detriment of our code.
>> +
>> +We also need to acknowledge that many of our developers are used to 
>> this
>> +mailing list workflow and have worked with it for years, with tools and
>> +processes built around it. Changing away from this would result in a 
>> loss
>> +of key people from the project, which would again be to its detriment.
>> +
>> +The projects are acutely aware that potential new contributors find the
>> +mailing list approach off-putting and would prefer a point-and-click 
>> web GUI.
>> +Since we don’t believe that can work for us, the project is aiming 
>> to ensure
>> +`patchwork <https://patchwork.yoctoproject.org/>` is available to 
>> help track
>> +patch status and also looking at how tooling can provide more 
>> feedback to users
>> +about patch status. We are looking at tools such as ``patchtest`` to
>> +test user contributions before they hit the mailing lists and also 
>> at better
>> +documenting how to use such workflows since we recognise that whilst 
>> this was
>> +common knowledge a decade ago, it might not be as familiar now.
>> +
>>   Finding a Suitable Mailing List
>>   ===============================
>>   -The Yocto Project uses a mailing list and a patch-based workflow 
>> that is
>
> Oops
>
>>   similar to the Linux kernel but contains important differences. In
>>   general, there is a mailing list through which you can submit 
>> patches. You
>>   should send patches to the appropriate mailing list so that they 
>> can be
>>
>>
Oops indeed. Fixed directly in master-next.
Thanks for the review.
Cheers
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH 2/3] contributor-guide: add section about why we use mailing lists

[Michael Opdenacker]

On 09.08.23 at 19:57, Richard Purdie wrote:
>>
>> I find the term "point-and-click" a little patronizing,
>> github/gitlab/others web tools
>> are not just point-and-click machine but highly customizable tool
>> aims at smoothen the developer work (in term of tracking, viewing
>> diff, forking) and to provide enhanced user experience.
> I agree we should tweak the wording.


I changed the text to a more neutral "would prefer a web-based GUI".
Done directly in master-next.
Thanks for the review!
Michael.


-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com