12:35 p.m.
Changes from [v1]:
* convert simple tables to definition lists;
* drop glib-adoption.rst entirely instead of tweaking it.
[v1] https://www.redhat.com/archives/libvir-list/2020-May/msg00292.html
Andrea Bolognani (3):
docs: Drop glib-adoption.rst
docs: Use definition list instead of table in coding style
docs: Document list-tables as recommended
docs/coding-style.rst | 47 ++++++++++++---------
docs/glib-adoption.rst | 96 ------------------------------------------
docs/hacking.rst | 1 -
docs/styleguide.rst | 20 +++++++++
4 files changed, 46 insertions(+), 118 deletions(-)
delete mode 100644 docs/glib-adoption.rst
--
2.25.4
12:35 p.m.
New subject: [libvirt PATCH v2 1/3] docs: Drop glib-adoption.rst
It's been more than six months since we adopted GLib and we've
been pretty aggressive at replacing our homegrown APIs with more
standard ones, so by now most of the symbols mentioned in this
document haven't been around for quite a long time already.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/glib-adoption.rst | 96 ------------------------------------------
docs/hacking.rst | 1 -
2 files changed, 97 deletions(-)
delete mode 100644 docs/glib-adoption.rst
diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
deleted file mode 100644
index 62ddd7c61d..0000000000
--- a/docs/glib-adoption.rst
+++ /dev/null
@@ -1,96 +0,0 @@
-=====================
-Adoption of GLib APIs
-=====================
-
-Libvirt has adopted use of the `GLib
-library <https://developer.gnome.org/glib/stable/>`__. Due to
-libvirt's long history of development, there are many APIs in
-libvirt, for which GLib provides an alternative solution. The
-general rule to follow is that the standard GLib solution will be
-preferred over historical libvirt APIs. Existing code will be
-ported over to use GLib APIs over time, but new code should use
-the GLib APIs straight away where possible.
-
-The following is a list of libvirt APIs that should no longer be
-used in new code, and their suggested GLib replacements:
-
-``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``, ``VIR_SHRINK_N``,
``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, ``VIR_DELETE_ELEMENT``
- Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
- cases. There should rarely be a need to use
- ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
- is preferrable to use one of the GLib types, ``GArray``,
- ``GPtrArray`` or ``GByteArray``. These all use a struct to
- track the array memory and size together and efficiently
- resize. **NEVER MIX** use of the classic libvirt memory
- allocation APIs and GLib APIs within a single method. Keep the
- style consistent, converting existing code to GLib style in a
- separate, prior commit.
-``virStrerror``
- The GLib ``g_strerror()`` function should be used instead,
- which has a simpler calling convention as an added benefit.
-
-The following libvirt APIs have been deleted already:
-
-``VIR_AUTOPTR``, ``VIR_AUTOCLEAN``, ``VIR_AUTOFREE``
- The GLib macros ``g_autoptr``, ``g_auto`` and ``g_autofree``
- must be used instead in all new code. In existing code, the
- GLib macros must never be mixed with libvirt macros within a
- method, nor should they be mixed with ``VIR_FREE``. If
- introducing GLib macros to an existing method, any use of
- libvirt macros must be converted in an independent commit.
-``VIR_DEFINE_AUTOPTR_FUNC``, ``VIR_DEFINE_AUTOCLEAN_FUNC``
- The GLib macros ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` and
- ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` must be used in all new
- code. Existing code should be converted to the new macros where
- relevant. It is permissible to use ``g_autoptr``, ``g_auto`` on
- an object whose cleanup function is declared with the libvirt
- macros and vice-versa.
-``VIR_AUTOUNREF``
- The GLib macros ``g_autoptr`` and
- ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` should be used to manage
- autoclean of virObject classes. This matches usage with GObject
- classes.
-``VIR_STRDUP``, ``VIR_STRNDUP``
- Prefer the GLib APIs ``g_strdup`` and ``g_strndup``.
-
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| deleted version | GLib version | Notes
|
-+===============================+======================================+===========================================+
-| ``VIR_AUTOPTR`` | ``g_autoptr`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_AUTOCLEAN`` | ``g_auto`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_AUTOFREE`` | ``g_autofree`` | The GLib version
does not use parentheses |
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_AUTOUNREF`` | ``g_autoptr`` | The cleanup
function needs to be defined |
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_DEFINE_AUTOPTR_FUNC`` | ``G_DEFINE_AUTOPTR_CLEANUP_FUNC`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_DEFINE_AUTOCLEAN_FUNC`` | ``G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_STEAL_PTR`` | ``g_steal_pointer`` | ``a =
f(&b)`` instead of ``f(a, b)`` |
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_RETURN_PTR`` | ``return g_steal_pointer`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ARRAY_CARDINALITY`` | ``G_N_ELEMENTS`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_FALLTHROUGH`` | ``G_GNUC_FALLTHROUGH`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_FMT_PRINTF`` | ``G_GNUC_PRINTF`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_NOINLINE`` | ``G_GNUC_NO_INLINE`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_NORETURN`` | ``G_GNUC_NORETURN`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_RETURN_CHECK`` | ``G_GNUC_WARN_UNUSED_RESULT`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_SENTINEL`` | ``G_GNUC_NULL_TERMINATED`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``ATTRIBUTE_UNUSED`` | ``G_GNUC_UNUSED`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_STRDUP`` | ``g_strdup`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``VIR_STRNDUP`` | ``g_strndup`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
-| ``virStrerror`` | ``g_strerror`` |
|
-+-------------------------------+--------------------------------------+-------------------------------------------+
diff --git a/docs/hacking.rst b/docs/hacking.rst
index 1c64146c5d..61ae6452e1 100644
--- a/docs/hacking.rst
+++ b/docs/hacking.rst
@@ -75,5 +75,4 @@ you also take a look at the following documents:
- `Programming languages <programming-languages.html>`__
- `Developer tooling <developer-tooling.html>`__
- `Advanced test suite usage <advanced-tests.html>`__
-- `Adoption of GLib APIs <glib-adoption.html>`__
- `Committer guidelines <committer-guidelines.html>`__
--
2.25.4
7:24 a.m.
New subject: [libvirt PATCH v2 1/3] docs: Drop glib-adoption.rst
On a Thursday in 2020, Andrea Bolognani wrote:
It's been more than six months since we adopted GLib and
we've
been pretty aggressive at replacing our homegrown APIs with more
standard ones, so by now most of the symbols mentioned in this
document haven't been around for quite a long time already.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/glib-adoption.rst | 96 ------------------------------------------
docs/hacking.rst | 1 -
2 files changed, 97 deletions(-)
delete mode 100644 docs/glib-adoption.rst
diff --git a/docs/glib-adoption.rst b/docs/glib-adoption.rst
deleted file mode 100644
index 62ddd7c61d..0000000000
--- a/docs/glib-adoption.rst
+++ /dev/null
@@ -1,96 +0,0 @@
-=====================
-Adoption of GLib APIs
-=====================
-
-Libvirt has adopted use of the `GLib
-library <https://developer.gnome.org/glib/stable/>`__. Due to
-libvirt's long history of development, there are many APIs in
-libvirt, for which GLib provides an alternative solution. The
-general rule to follow is that the standard GLib solution will be
-preferred over historical libvirt APIs. Existing code will be
-ported over to use GLib APIs over time, but new code should use
-the GLib APIs straight away where possible.
-
-The following is a list of libvirt APIs that should no longer be
-used in new code, and their suggested GLib replacements:
-
-``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``, ``VIR_SHRINK_N``,
``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``, ``VIR_DELETE_ELEMENT``
- Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
- cases. There should rarely be a need to use
- ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
This is the only place where the preferred GLib functions are
documented, I think deleting it is premature.
Jano
- is preferrable to use one of the GLib types, ``GArray``,
- ``GPtrArray`` or ``GByteArray``. These all use a struct to
- track the array memory and size together and efficiently
- resize. **NEVER MIX** use of the classic libvirt memory
- allocation APIs and GLib APIs within a single method. Keep the
- style consistent, converting existing code to GLib style in a
- separate, prior commit.
-``virStrerror``
- The GLib ``g_strerror()`` function should be used instead,
- which has a simpler calling convention as an added benefit.
7:35 a.m.
New subject: [libvirt PATCH v2 1/3] docs: Drop glib-adoption.rst
On Mon, 2020-05-11 at 14:24 +0200, Ján Tomko wrote:
On a Thursday in 2020, Andrea Bolognani wrote:
> -The following is a list of libvirt APIs that should no longer be
> -used in new code, and their suggested GLib replacements:
> -
> -``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``,
``VIR_SHRINK_N``, ``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``,
``VIR_DELETE_ELEMENT``
> - Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
> - cases. There should rarely be a need to use
> - ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
This is the only place where the preferred GLib functions are
documented, I think deleting it is premature.
The patch has already been merged.
I think regular contributors have become used to the GLib APIs by
now, and drive-by contributors were probably not familiar with the
libvirt APIs in the first place, so this list was of no use to them.
That said, if you think there's value in keeping this document
around, you can definitely post a revert patch and see where that
goes :)
--
Andrea Bolognani / Red Hat / Virtualization
7:42 a.m.
New subject: [libvirt PATCH v2 1/3] docs: Drop glib-adoption.rst
On Mon, May 11, 2020 at 02:35:33PM +0200, Andrea Bolognani wrote:
On Mon, 2020-05-11 at 14:24 +0200, Ján Tomko wrote:
> On a Thursday in 2020, Andrea Bolognani wrote:
> > -The following is a list of libvirt APIs that should no longer be
> > -used in new code, and their suggested GLib replacements:
> > -
> > -``VIR_ALLOC``, ``VIR_REALLOC``, ``VIR_RESIZE_N``, ``VIR_EXPAND_N``,
``VIR_SHRINK_N``, ``VIR_FREE``, ``VIR_APPEND_ELEMENT``, ``VIR_INSERT_ELEMENT``,
``VIR_DELETE_ELEMENT``
> > - Prefer the GLib APIs ``g_new0``/``g_renew``/ ``g_free`` in most
> > - cases. There should rarely be a need to use
> > - ``g_malloc``/``g_realloc``. Instead of using plain C arrays, it
>
> This is the only place where the preferred GLib functions are
> documented, I think deleting it is premature.
It is also documented in the viralloc.h header
The patch has already been merged.
I think regular contributors have become used to the GLib APIs by
now, and drive-by contributors were probably not familiar with the
libvirt APIs in the first place, so this list was of no use to them.
We're still at a 10:1 ratio of VIR_ALLOC:g_new0 which suprised me a
bit.
We were quite succesful with our big push to convert other areas of
code to GLib APIs. eg the ATTRIBUTE_*. Admittedly these were simpler
cases, but we could benefit from being a bit more aggressive in
eliminated VIR_ALLOC related APIs at least.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
12:35 p.m.
New subject: [libvirt PATCH v2 2/3] docs: Use definition list instead of table in coding style
This format is much easier to tweak and update.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/coding-style.rst | 47 ++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index 151ea87b6a..03b89c86e5 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -547,27 +547,32 @@ Attribute annotations
Use the following annotations to help the compiler and/or static
analysis tools understand the code better:
-+-------------------------------+------------------------------------------------------------+
-| Macro | Meaning
|
-+===============================+============================================================+
-| ``ATTRIBUTE_NONNULL`` | passing NULL for this parameter is not allowed
|
-+-------------------------------+------------------------------------------------------------+
-| ``ATTRIBUTE_PACKED`` | force a structure to be packed
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_FALLTHROUGH`` | allow code reuse by multiple switch cases
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_NO_INLINE`` | the function is mocked in the test suite
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_NORETURN`` | the function never returns
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_NULL_TERMINATED`` | last parameter must be NULL
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_PRINTF`` | validate that the formatting string matches parameters
|
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_UNUSED`` | parameter is unused in this implementation of the
function |
-+-------------------------------+------------------------------------------------------------+
-| ``G_GNUC_WARN_UNUSED_RESULT`` | the return value must be checked
|
-+-------------------------------+------------------------------------------------------------+
+``ATTRIBUTE_NONNULL``
+ passing NULL for this parameter is not allowed
+
+``ATTRIBUTE_PACKED``
+ force a structure to be packed
+
+``G_GNUC_FALLTHROUGH``
+ allow code reuse by multiple switch cases
+
+``G_GNUC_NO_INLINE``
+ the function is mocked in the test suite
+
+``G_GNUC_NORETURN``
+ the function never returns
+
+``G_GNUC_NULL_TERMINATED``
+ last parameter must be NULL
+
+``G_GNUC_PRINTF``
+ validate that the formatting string matches parameters
+
+``G_GNUC_UNUSED``
+ parameter is unused in this implementation of the function
+
+``G_GNUC_WARN_UNUSED_RESULT``
+ the return value must be checked
File handling
-------------
--
2.25.4
12:35 p.m.
New subject: [libvirt PATCH v2 3/3] docs: Document list-tables as recommended
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/styleguide.rst | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/docs/styleguide.rst b/docs/styleguide.rst
index 3162868fb7..34c5b1573a 100644
--- a/docs/styleguide.rst
+++ b/docs/styleguide.rst
@@ -77,6 +77,26 @@ which allows for 6 levels of headings
Heading 6
^^^^^^^^^
+Tables
+======
+
+Tables should be created using the ``list-table`` directive whenever
+possible, as in
+
+::
+
+ .. list-table::
+ :header-rows: 1
+
+ * - Option
+ - Description
+
+ * - ``foo_enabled``
+ - Whether or not ``foo`` should be enabled
+
+ * - ``bar_user``
+ - Which user to run ``bar`` as
+
Manual pages
============
--
2.25.4
4:39 a.m.
On Thu, May 07, 2020 at 07:35:14PM +0200, Andrea Bolognani wrote:
Changes from [v1]:
* convert simple tables to definition lists;
* drop glib-adoption.rst entirely instead of tweaking it.
[v1] https://www.redhat.com/archives/libvir-list/2020-May/msg00292.html
Andrea Bolognani (3):
docs: Drop glib-adoption.rst
docs: Use definition list instead of table in coding style
docs: Document list-tables as recommended
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1656
days inactive
1660
days old
7 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Ján Tomko