Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 03:14:38PM +0100, Daniel P. Berrangé wrote: > On Thu, Aug 23, 2018 at 03:55:16PM +0200, Peter Krempa wrote: > > On Thu, Aug 23, 2018 at 15:46:30 +0200, Erik Skultety wrote: > > > On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: > > > > The pathches used as an example for the api_extension manual don't hold > > > > > > s/pathches/patches > > > > > > > up to the current standards any more. Carefully remove links and > > > > mentions of the patches from the docs. > > > > > > While I understand the impetus for the change, I am personally not > > > convinced > > > that we want to get rid of practical examples as a means of reference to > > > the > > > text, it's like a product documentation without examples - "hardcore > > > mode". > > > > Any example will always become obsolete. I find trying to update it to > > be a waste of time. > > I think that is rather a self fullfilling prophecy. > > The example becomes obsolete because no one can be bothered to spend > time updating it. This doesn't imply we shouldn't even try. IMHO it > is largely a sign that our priorities are screwed up, and we should > put more effort into non-code writing parts of the project. > > Creating a long term healthy & viable project needs more than just > writing lots of code. Considering feature implementation alone, > there needs to be unit testing of the work, integration testing of > the bigger system, documentation of the APIs and/or knowledgebase > tutorials showing usage. In fact actually writing the core functional > code could arguably end up being quite a small part of the overall > work on a feature. > > Feature work is only one part of the project's long term success > though. Bringing onboard new contributors is a key factor, and having > something more to say to novices than "go read the code" is important > to smoothing their onramp. Teaching application developers & admins > what we've provide and how to use it is also heavily neglected in > most cases such that we have great features no one knows about or > uses correctly. > > Unfortunately we've historically not been very good at much of this, > being very tightly focused on just writing the core code. This has > definitely harmed our success in many areas. For example application > developers have frequently questioned what value libvirt adds because > we've not done a good enough job of telling people what we have > implemented, or how to use it correctly once it exists. > > > > The fact that we took a patch series from the list archives as a reference > > > probably wasn't the best thing to do, I think we should instead come up > > > with a > > > dummy API exercising all the sections in the docs which will conform to > > > our > > > > Even coding style will become obsolete. Just the fact that we started > > using the autofree stuff will make many existing examples obsolete. > > > > > current standards and which we can update as we please. I know that's > > > much more > > > > We don't even keep our coding style guidelines up to date most of the > > time. Do you think this would be any different? > > This is a failure of our review process. In adding new code style rules, > we should consider updating of examples as required part of the work. > Reviewers should raise this if it isn't done. I'm as guilty of missing > this as the next person, but we shouldn't give up otherwise we will just > stagnate. > > IOW, lack of updated examples wrt autofree is a bug with the original > autofree patch series we missed. Right, but then again, we preferred converting the code base in batches. Nevertheless, we can update the guidelines for sure. Apart from that, couldn't agree more with what you wrote. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 03:55:16PM +0200, Peter Krempa wrote: > On Thu, Aug 23, 2018 at 15:46:30 +0200, Erik Skultety wrote: > > On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: > > > The pathches used as an example for the api_extension manual don't hold > > > > s/pathches/patches > > > > > up to the current standards any more. Carefully remove links and > > > mentions of the patches from the docs. > > > > While I understand the impetus for the change, I am personally not convinced > > that we want to get rid of practical examples as a means of reference to the > > text, it's like a product documentation without examples - "hardcore mode". > > Any example will always become obsolete. I find trying to update it to > be a waste of time. I think that is rather a self fullfilling prophecy. The example becomes obsolete because no one can be bothered to spend time updating it. This doesn't imply we shouldn't even try. IMHO it is largely a sign that our priorities are screwed up, and we should put more effort into non-code writing parts of the project. Creating a long term healthy & viable project needs more than just writing lots of code. Considering feature implementation alone, there needs to be unit testing of the work, integration testing of the bigger system, documentation of the APIs and/or knowledgebase tutorials showing usage. In fact actually writing the core functional code could arguably end up being quite a small part of the overall work on a feature. Feature work is only one part of the project's long term success though. Bringing onboard new contributors is a key factor, and having something more to say to novices than "go read the code" is important to smoothing their onramp. Teaching application developers & admins what we've provide and how to use it is also heavily neglected in most cases such that we have great features no one knows about or uses correctly. Unfortunately we've historically not been very good at much of this, being very tightly focused on just writing the core code. This has definitely harmed our success in many areas. For example application developers have frequently questioned what value libvirt adds because we've not done a good enough job of telling people what we have implemented, or how to use it correctly once it exists. > > The fact that we took a patch series from the list archives as a reference > > probably wasn't the best thing to do, I think we should instead come up > > with a > > dummy API exercising all the sections in the docs which will conform to our > > Even coding style will become obsolete. Just the fact that we started > using the autofree stuff will make many existing examples obsolete. > > > current standards and which we can update as we please. I know that's much > > more > > We don't even keep our coding style guidelines up to date most of the > time. Do you think this would be any different? This is a failure of our review process. In adding new code style rules, we should consider updating of examples as required part of the work. Reviewers should raise this if it isn't done. I'm as guilty of missing this as the next person, but we shouldn't give up otherwise we will just stagnate. IOW, lack of updated examples wrt autofree is a bug with the original autofree patch series we missed. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 03:55:16PM +0200, Peter Krempa wrote: > On Thu, Aug 23, 2018 at 15:46:30 +0200, Erik Skultety wrote: > > On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: > > > The pathches used as an example for the api_extension manual don't hold > > > > s/pathches/patches > > > > > up to the current standards any more. Carefully remove links and > > > mentions of the patches from the docs. > > > > While I understand the impetus for the change, I am personally not convinced > > that we want to get rid of practical examples as a means of reference to the > > text, it's like a product documentation without examples - "hardcore mode". > > Any example will always become obsolete. I find trying to update it to > be a waste of time. > > Users are always welcome to inspire from any existing piece of code. We > are open-source in the end. I don't really think we need as much > hand-holding in this area. > > > The fact that we took a patch series from the list archives as a reference > > probably wasn't the best thing to do, I think we should instead come up > > with a > > dummy API exercising all the sections in the docs which will conform to our > > Even coding style will become obsolete. Just the fact that we started > using the autofree stuff will make many existing examples obsolete. > > > current standards and which we can update as we please. I know that's much > > more > > We don't even keep our coding style guidelines up to date most of the > time. Do you think this would be any different? That's a good point. In fact, not just that, once we split the daemon into multiple daemons, it's likely that the guide itself might need updating, let's see how that will turn out. You already got a RB from Jano and since I'm undecided here, whatever. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: The pathches used as an example for the api_extension manual don't hold *patches, as Erik pointed out up to the current standards any more. Carefully remove links and mentions of the patches from the docs. Signed-off-by: Peter Krempa --- docs/api_extension.html.in | 96 +- 1 file changed, 18 insertions(+), 78 deletions(-) @@ -197,22 +178,16 @@ The public API calls are implemented in: -src/libvirt.c - -See 0004-implement-the-public-APIs.patch +src/libvirt-$SECTION.c Unrelated change from libvirt.c to libvirt-$SECTION.c Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 15:46:30 +0200, Erik Skultety wrote: > On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: > > The pathches used as an example for the api_extension manual don't hold > > s/pathches/patches > > > up to the current standards any more. Carefully remove links and > > mentions of the patches from the docs. > > While I understand the impetus for the change, I am personally not convinced > that we want to get rid of practical examples as a means of reference to the > text, it's like a product documentation without examples - "hardcore mode". Any example will always become obsolete. I find trying to update it to be a waste of time. Users are always welcome to inspire from any existing piece of code. We are open-source in the end. I don't really think we need as much hand-holding in this area. > The fact that we took a patch series from the list archives as a reference > probably wasn't the best thing to do, I think we should instead come up with a > dummy API exercising all the sections in the docs which will conform to our Even coding style will become obsolete. Just the fact that we started using the autofree stuff will make many existing examples obsolete. > current standards and which we can update as we please. I know that's much > more We don't even keep our coding style guidelines up to date most of the time. Do you think this would be any different? > work and effort which I can see you'd refuse to put into this, but I fear that > once you drop this, we won't update the docs with another examples for > years/ever and I'm not in favour of such practice. No. I'll drop the series if it does not get accepted and continue ignoring this as I've did for many years. I wanted to kill this for some time (when I was adding syntax-check rule and the patches were conflicting), but did not even want to bother to do that for at that time so I've just opted to skip them. Recent Eric's usage of the patches as an example in his mail discussing renaming of the files triggered me to get rid of this finally. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
On Thu, Aug 23, 2018 at 10:50:30AM +0200, Peter Krempa wrote: > The pathches used as an example for the api_extension manual don't hold s/pathches/patches > up to the current standards any more. Carefully remove links and > mentions of the patches from the docs. While I understand the impetus for the change, I am personally not convinced that we want to get rid of practical examples as a means of reference to the text, it's like a product documentation without examples - "hardcore mode". The fact that we took a patch series from the list archives as a reference probably wasn't the best thing to do, I think we should instead come up with a dummy API exercising all the sections in the docs which will conform to our current standards and which we can update as we please. I know that's much more work and effort which I can see you'd refuse to put into this, but I fear that once you drop this, we won't update the docs with another examples for years/ever and I'm not in favour of such practice. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] docs: api_extension: Remove links to the stale example patches
The pathches used as an example for the api_extension manual don't hold up to the current standards any more. Carefully remove links and mentions of the patches from the docs. Signed-off-by: Peter Krempa --- docs/api_extension.html.in | 96 +- 1 file changed, 18 insertions(+), 78 deletions(-) diff --git a/docs/api_extension.html.in b/docs/api_extension.html.in index 9beec07602..b473d09b17 100644 --- a/docs/api_extension.html.in +++ b/docs/api_extension.html.in @@ -8,14 +8,9 @@ This document walks you through the process of implementing a new - API in libvirt. It uses as an example the addition of an API for - separating maximum from current vcpu usage of a domain, over - the course of a fifteen-patch series. - Remember that new API consists of any new public functions, as - well as the addition of flags or extensions of XML used by - existing functions. The example in this document adds both new - functions and an XML extension. Not all libvirt API additions - require quite as many patches. + API in libvirt. Remember that new API consists of any new public + functions, as well as the addition of flags or extensions of XML used by + existing functions. @@ -27,12 +22,7 @@ added to libvirt. Someone may already be working on the feature you want. Also, recognize that everything you write is likely to undergo significant rework as you discuss it with the other developers, so - don't wait too long before getting feedback. In the vcpu example - below, list feedback was first requested - https://www.redhat.com/archives/libvir-list/2010-September/msg00423.html;>here - and resulted in several rounds of improvements before coding - began. In turn, this example is slightly rearranged from the actual - order of the commits. + don't wait too long before getting feedback. @@ -81,14 +71,13 @@ - Submit new code in the form shown in the example code: one patch - per step. That's not to say submit patches before you have working - functionality--get the whole thing working and make sure you're happy - with it. Then use git or some other version control system that lets - you rewrite your commit history and break patches into pieces so you - don't drop a big blob of code on the mailing list in one go. - Also, you should follow the upstream tree, and rebase your - series to adapt your patches to work with any other changes + Submit new code in the form of one patch per step. That's not to say + submit patches before you have working functionality--get the whole thing + working and make sure you're happy with it. Then use git or some other + version control system that lets you rewrite your commit history and + break patches into pieces so you don't drop a big blob of code on the + mailing list in one go. Also, you should follow the upstream tree, and + rebase your series to adapt your patches to work with any other changes that were accepted upstream during your development. @@ -101,8 +90,6 @@ separately. -With that said, let's begin. - Defining the public API The first task is to define the public API. If the new API @@ -133,10 +120,6 @@ rework it as you go through the process of implementing it. -See 0001-add-to-xml.patch -and 0002-add-new-public-API.patch -for example code. - Defining the internal API @@ -164,8 +147,6 @@ provide a NULL stub for the new function. -See 0003-define-internal-driver-API.patch - Implementing the public API @@ -197,22 +178,16 @@ The public API calls are implemented in: -src/libvirt.c - -See 0004-implement-the-public-APIs.patch +src/libvirt-$SECTION.c Implementing the remote protocol Implementing the remote protocol is essentially a straightforward exercise which is probably most easily - understood by referring to the existing code and the example - patch. It involves several related changes, including the - regeneration of derived files, with further details below. + understood by referring to the existing code. -See 0005-implement-the-remote-protocol.patch - Defining the wire protocol format @@ -298,8 +273,6 @@ existing lines probably imply a backwards-incompatible API change. -See 0005-implement-the-remote-protocol.patch - Use the new API internally @@ -312,8 +285,6 @@ not necessary if the new API has no relation to existing API. -See 0006-make-old-API-trivially-wrap-to-new-API.patch - Expose the new API in virsh @@ -343,8 +314,6 @@ tools/virsh.pod -See 0007-add-virsh-support.patch - Implement the driver methods
