| Message ID | 20250921164306.53501-2-bhstalel@gmail.com |
|---|---|
| State | Under Review |
| Headers | show |
| Series | Add more details about the behaviour of the base class | expand |
On Sun Sep 21, 2025 at 6:43 PM CEST, BELHADJ SALEM Talel via lists.yoctoproject.org wrote: > Signed-off-by: Talel BELHAJ SALEM <bhstalel@gmail.com> > --- > documentation/ref-manual/classes.rst | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst > index 662121ed9..791c6b2ef 100644 > --- a/documentation/ref-manual/classes.rst > +++ b/documentation/ref-manual/classes.rst > @@ -183,6 +183,14 @@ The class also contains some commonly used functions such as > :term:`EXTRA_OEMAKE` variable as well as the > arguments passed directly to ``oe_runmake``. > > +The following actions, among others, are handled by this class: I would perhaps phrase this differently, how about: """ For example, the following items are handled by this class: """ So intent is more about giving some examples about what the class is doing. > + > +- Define default dependencies and add them to :term:`DEPENDS` if :term:`INHIBIT_DEFAULT_DEPS` > + is not defined What dependencies? I'm afraid adding this sentence may add confusion instead of adding precision. > +- Handle :term:`PACKAGECONFIG` variable This is vague too. I would extend this sentence to briefly give some context. > +- Handle the provided license via :term:`LICENSE` and :term:`LIC_FILES_CHKSUM` variables s/license/license(s) This is again a bit vague, I'm not sure what's the added value. How about: """ Handle the provided license(s) specified in the :term:`LICENSE` variable, and verify its checksum specified in the :term:`LIC_FILES_CHKSUM` variable. """ > +- Prepare the native recipe that correspond to the scheme set in :term:`SRC_URI` This is too vague, I'm not sure what you wanted to convey here. > + > .. _ref-classes-bash-completion: > > ``bash-completion`` I would finish by saying: """ See the source code of the :ref:`ref-classes-base` class to get a complete overview of what the class handles. """ So that way we insist on the fact that we were just giving examples. Thanks, Antonin
I totally agree with you. I was sure that it is too vague, I don't why I did not detailed it more clearly. I will add more details in v2 Thanks On Mon, 22 Sep 2025 at 15:32 Antonin Godard <antonin.godard@bootlin.com> wrote: > On Sun Sep 21, 2025 at 6:43 PM CEST, BELHADJ SALEM Talel via > lists.yoctoproject.org wrote: > > Signed-off-by: Talel BELHAJ SALEM <bhstalel@gmail.com> > > --- > > documentation/ref-manual/classes.rst | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/documentation/ref-manual/classes.rst > b/documentation/ref-manual/classes.rst > > index 662121ed9..791c6b2ef 100644 > > --- a/documentation/ref-manual/classes.rst > > +++ b/documentation/ref-manual/classes.rst > > @@ -183,6 +183,14 @@ The class also contains some commonly used > functions such as > > :term:`EXTRA_OEMAKE` variable as well as the > > arguments passed directly to ``oe_runmake``. > > > > +The following actions, among others, are handled by this class: > > I would perhaps phrase this differently, how about: > > """ > For example, the following items are handled by this class: > """ > > So intent is more about giving some examples about what the class is doing. > > > + > > +- Define default dependencies and add them to :term:`DEPENDS` if > :term:`INHIBIT_DEFAULT_DEPS` > > + is not defined > > What dependencies? I'm afraid adding this sentence may add confusion > instead of > adding precision. > > > +- Handle :term:`PACKAGECONFIG` variable > > This is vague too. I would extend this sentence to briefly give some > context. > > > +- Handle the provided license via :term:`LICENSE` and > :term:`LIC_FILES_CHKSUM` variables > > s/license/license(s) > > This is again a bit vague, I'm not sure what's the added value. How about: > > """ > Handle the provided license(s) specified in the :term:`LICENSE` variable, > and > verify its checksum specified in the :term:`LIC_FILES_CHKSUM` variable. > """ > > > +- Prepare the native recipe that correspond to the scheme set in > :term:`SRC_URI` > > This is too vague, I'm not sure what you wanted to convey here. > > > + > > .. _ref-classes-bash-completion: > > > > ``bash-completion`` > > I would finish by saying: > > """ > See the source code of the :ref:`ref-classes-base` class to get a complete > overview of what the class handles. > """ > > So that way we insist on the fact that we were just giving examples. > > Thanks, > Antonin > > -- > Antonin Godard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > >
I will wait for your review on the 2/2 patch before submitting v2. Kind Regards Talel On Mon, Sep 22, 2025 at 7:06 PM BELHADJ SALEM Talel via lists.yoctoproject.org <bhstalel=gmail.com@lists.yoctoproject.org> wrote: > I totally agree with you. > > I was sure that it is too vague, I don't why I did not detailed it more > clearly. > > I will add more details in v2 > > Thanks > > On Mon, 22 Sep 2025 at 15:32 Antonin Godard <antonin.godard@bootlin.com> > wrote: > >> On Sun Sep 21, 2025 at 6:43 PM CEST, BELHADJ SALEM Talel via >> lists.yoctoproject.org wrote: >> > Signed-off-by: Talel BELHAJ SALEM <bhstalel@gmail.com> >> > --- >> > documentation/ref-manual/classes.rst | 8 ++++++++ >> > 1 file changed, 8 insertions(+) >> > >> > diff --git a/documentation/ref-manual/classes.rst >> b/documentation/ref-manual/classes.rst >> > index 662121ed9..791c6b2ef 100644 >> > --- a/documentation/ref-manual/classes.rst >> > +++ b/documentation/ref-manual/classes.rst >> > @@ -183,6 +183,14 @@ The class also contains some commonly used >> functions such as >> > :term:`EXTRA_OEMAKE` variable as well as the >> > arguments passed directly to ``oe_runmake``. >> > >> > +The following actions, among others, are handled by this class: >> >> I would perhaps phrase this differently, how about: >> >> """ >> For example, the following items are handled by this class: >> """ >> >> So intent is more about giving some examples about what the class is >> doing. >> >> > + >> > +- Define default dependencies and add them to :term:`DEPENDS` if >> :term:`INHIBIT_DEFAULT_DEPS` >> > + is not defined >> >> What dependencies? I'm afraid adding this sentence may add confusion >> instead of >> adding precision. >> >> > +- Handle :term:`PACKAGECONFIG` variable >> >> This is vague too. I would extend this sentence to briefly give some >> context. >> >> > +- Handle the provided license via :term:`LICENSE` and >> :term:`LIC_FILES_CHKSUM` variables >> >> s/license/license(s) >> >> This is again a bit vague, I'm not sure what's the added value. How about: >> >> """ >> Handle the provided license(s) specified in the :term:`LICENSE` variable, >> and >> verify its checksum specified in the :term:`LIC_FILES_CHKSUM` variable. >> """ >> >> > +- Prepare the native recipe that correspond to the scheme set in >> :term:`SRC_URI` >> >> This is too vague, I'm not sure what you wanted to convey here. >> >> > + >> > .. _ref-classes-bash-completion: >> > >> > ``bash-completion`` >> >> I would finish by saying: >> >> """ >> See the source code of the :ref:`ref-classes-base` class to get a complete >> overview of what the class handles. >> """ >> >> So that way we insist on the fact that we were just giving examples. >> >> Thanks, >> Antonin >> >> -- >> Antonin Godard, Bootlin >> Embedded Linux and Kernel engineering >> https://bootlin.com >> >> > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#7575): > https://lists.yoctoproject.org/g/docs/message/7575 > Mute This Topic: https://lists.yoctoproject.org/mt/115361433/1955228 > Group Owner: docs+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/docs/unsub [ > bhstalel@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
On Mon Sep 22, 2025 at 8:12 PM CEST, Talel BELHADJ SALEM wrote: > I will wait for your review on the 2/2 patch before submitting v2. > > Kind Regards > Talel Feel free to send the first one separately. It's not strictly related to the second one if I understood correctly. Reviewing the diagram will take a bit more time. One remark I just had is that we would need to stick with the Yocto Project color scheme, fonts, etc. See: https://git.yoctoproject.org/yocto-docs/tree/documentation/standards.md#n173 Thanks, Antonin
Hello Antonin I fixed fonts and size of the diagram, regarding the colors palette, I am not sure how to do that in draw.io but I kept things simple and used only black, grey and white. I am sending a v2 On Tue, Sep 23, 2025 at 5:15 PM Antonin Godard <antonin.godard@bootlin.com> wrote: > On Mon Sep 22, 2025 at 8:12 PM CEST, Talel BELHADJ SALEM wrote: > > I will wait for your review on the 2/2 patch before submitting v2. > > > > Kind Regards > > Talel > > Feel free to send the first one separately. It's not strictly related to > the > second one if I understood correctly. > > Reviewing the diagram will take a bit more time. > > One remark I just had is that we would need to stick with the Yocto Project > color scheme, fonts, etc. See: > > https://git.yoctoproject.org/yocto-docs/tree/documentation/standards.md#n173 > > Thanks, > Antonin > > -- > Antonin Godard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > >
On Thu Sep 25, 2025 at 4:55 PM CEST, Talel BELHADJ SALEM wrote: > Hello Antonin > > I fixed fonts and size of the diagram, regarding the colors palette, I am > not sure how to do that in draw.io > but I kept things simple and used only black, grey and white. > I am sending a v2 Hi, Thanks. Please see my comments on the diagram in the v1 thread, that I sent ~40minutes ago. Antonin
diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst index 662121ed9..791c6b2ef 100644 --- a/documentation/ref-manual/classes.rst +++ b/documentation/ref-manual/classes.rst @@ -183,6 +183,14 @@ The class also contains some commonly used functions such as :term:`EXTRA_OEMAKE` variable as well as the arguments passed directly to ``oe_runmake``. +The following actions, among others, are handled by this class: + +- Define default dependencies and add them to :term:`DEPENDS` if :term:`INHIBIT_DEFAULT_DEPS` + is not defined +- Handle :term:`PACKAGECONFIG` variable +- Handle the provided license via :term:`LICENSE` and :term:`LIC_FILES_CHKSUM` variables +- Prepare the native recipe that correspond to the scheme set in :term:`SRC_URI` + .. _ref-classes-bash-completion: ``bash-completion``
Signed-off-by: Talel BELHAJ SALEM <bhstalel@gmail.com> --- documentation/ref-manual/classes.rst | 8 ++++++++ 1 file changed, 8 insertions(+)