* Xen Coding style @ 2020-05-07 14:43 Julien Grall 2020-05-08 8:36 ` Jan Beulich 0 siblings, 1 reply; 11+ messages in thread From: Julien Grall @ 2020-05-07 14:43 UTC (permalink / raw) To: committers; +Cc: xen-devel, Woodhouse, David, paul Hi all, This was originally discussed during the last community call. A major part of the comments during review is related to coding style issues. This can lead to frustration from the contributor as the current CODING_STYLE is quite light and the choice often down to a matter of taste from the maintainers. In the past, Lars tried to address the problem by introducing a coding style checker (see [1] and [2]). However, the work came to stop because we couldn't agree on what is Xen coding style. I would like to suggest a different approach. Rather than trying to agree on all the rules one by one, I propose to have a vote on different coding styles and pick the preferred one. The list of coding styles would come from the community. It could be coding styles already used in the open source community or new coding styles (for instance a contributor could write down his/her understanding of Xen Coding Style). Are the committers happy with this appraoch? If so, I could send a formal call for coding style? Cheers, [1] <20190718144317.23307-1-tamas@tklengyel.com> [2] <CAOcoXZavLnHhNc7mmHnO5Gi_vq-0j1FCgvpXh0NimAewXX8e1g@mail.gmail.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-07 14:43 Xen Coding style Julien Grall @ 2020-05-08 8:36 ` Jan Beulich 2020-05-08 10:00 ` Julien Grall 0 siblings, 1 reply; 11+ messages in thread From: Jan Beulich @ 2020-05-08 8:36 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, committers, Woodhouse, David, paul On 07.05.2020 16:43, Julien Grall wrote: > This was originally discussed during the last community call. > > A major part of the comments during review is related to coding style issues. This can lead to frustration from the contributor as the current CODING_STYLE is quite light and the choice often down to a matter of taste from the maintainers. > > In the past, Lars tried to address the problem by introducing a coding style checker (see [1] and [2]). However, the work came to stop because we couldn't agree on what is Xen coding style. > > I would like to suggest a different approach. Rather than trying to agree on all the rules one by one, I propose to have a vote on different coding styles and pick the preferred one. > > The list of coding styles would come from the community. It could be coding styles already used in the open source community or new coding styles (for instance a contributor could write down his/her understanding of Xen Coding Style). > > Are the committers happy with this appraoch? I'm not, sorry, and I'm pretty sure I indicated so before. For one I don't think picking an arbitrary other style than what we currently use is going to be helpful: It'll be a significant amount of code churn all over the code base. Instead I think the basic current principle should remain: Imported files, if not heavily changed, are permitted to retain their original style, while "our" files get written in "our" style. (Recording which one's which is still tbd.) I don't think though this necessarily implies "to agree on all the rules one by one" - we could also settle on there explicitly being freedom beyond what's spelled out explicitly. I'd not be happy with this, as it would lead to a lot of inconsistencies over time, but it's still an option. Obviously there's the wide range of middle ground to agree on some more rules to become written down (I did propose a few over time, without - iirc - getting helpful, if any, feedback), while leaving the rest up to the author. The main thing we need to settle on imo is whether unwritten rules count. Me being picky isn't because of me liking to be, but because of me thinking that a consistent code base is quite helpful. If consensus is that consistency is not a goal, I'll accept this (with some grumbling). Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 8:36 ` Jan Beulich @ 2020-05-08 10:00 ` Julien Grall 2020-05-08 11:20 ` Jan Beulich 0 siblings, 1 reply; 11+ messages in thread From: Julien Grall @ 2020-05-08 10:00 UTC (permalink / raw) To: Jan Beulich, Julien Grall; +Cc: xen-devel, committers, Woodhouse, David, paul On 08/05/2020 09:36, Jan Beulich wrote: > On 07.05.2020 16:43, Julien Grall wrote: >> This was originally discussed during the last community call. >> >> A major part of the comments during review is related to coding style issues. This can lead to frustration from the contributor as the current CODING_STYLE is quite light and the choice often down to a matter of taste from the maintainers. >> >> In the past, Lars tried to address the problem by introducing a coding style checker (see [1] and [2]). However, the work came to stop because we couldn't agree on what is Xen coding style. >> >> I would like to suggest a different approach. Rather than trying to agree on all the rules one by one, I propose to have a vote on different coding styles and pick the preferred one. >> >> The list of coding styles would come from the community. It could be coding styles already used in the open source community or new coding styles (for instance a contributor could write down his/her understanding of Xen Coding Style). >> >> Are the committers happy with this appraoch? > > I'm not, sorry, and I'm pretty sure I indicated so before. For one I > don't think picking an arbitrary other style than what we currently use > is going to be helpful: It'll be a significant amount of code churn all > over the code base. Instead I think the basic current principle should > remain: Imported files, if not heavily changed, are permitted to retain > their original style, while "our" files get written in "our" style. > (Recording which one's which is still tbd.) There are existing coding styles that are quite close to the one used by Xen (such as BSD). We could either use them as-is or make small changes to fit Xen. > > I don't think though this necessarily implies "to agree on all the rules > one by one" - we could also settle on there explicitly being freedom > beyond what's spelled out explicitly. I'd not be happy with this, as it > would lead to a lot of inconsistencies over time, but it's still an > option. Obviously there's the wide range of middle ground to agree on > some more rules to become written down (I did propose a few over time, > without - iirc - getting helpful, if any, feedback), while leaving the > rest up to the author. > > The main thing we need to settle on imo is whether unwritten rules > count. Me being picky isn't because of me liking to be, but because of > me thinking that a consistent code base is quite helpful. If consensus > is that consistency is not a goal, I'll accept this (with some > grumbling). Consistency is important, but this should not be based on unwritten rules. We should be able to write a script that can check whether a patch contain any violations. The end goal is to reduce the workload on the reviewer and the tension within the community. You seem to be the maintainer with the most unwritten rules. Would you mind to have a try at writing a coding style based on it? Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 10:00 ` Julien Grall @ 2020-05-08 11:20 ` Jan Beulich 2020-05-08 12:19 ` Julien Grall 0 siblings, 1 reply; 11+ messages in thread From: Jan Beulich @ 2020-05-08 11:20 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Julien Grall, committers, Woodhouse, David, paul On 08.05.2020 12:00, Julien Grall wrote: > You seem to be the maintainer with the most unwritten rules. Would > you mind to have a try at writing a coding style based on it? On the basis that even small, single aspect patches to CODING_STYLE have been ignored [1], I don't think this would be a good use of my time. If I was promised (reasonable) feedback, I could take what I have and try to add at least a few more things based on what I find myself commenting on more frequently. But really I'd prefer it to be done the other way around - for people to look at the patches already sent, and for me to only subsequently send more. After all, if already those adjustments are controversial, I don't think we could settle on others. Jan [1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01234.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 11:20 ` Jan Beulich @ 2020-05-08 12:19 ` Julien Grall 2020-05-08 12:55 ` Tamas K Lengyel 0 siblings, 1 reply; 11+ messages in thread From: Julien Grall @ 2020-05-08 12:19 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Julien Grall, committers, Woodhouse, David, paul Hi Jan, On 08/05/2020 12:20, Jan Beulich wrote: > On 08.05.2020 12:00, Julien Grall wrote: >> You seem to be the maintainer with the most unwritten rules. Would >> you mind to have a try at writing a coding style based on it? > > On the basis that even small, single aspect patches to CODING_STYLE > have been ignored [1], Your thread is one of the example why I started this thread. Agreeing on specific rule doesn't work because it either result to bikesheding or there is not enough interest to review rule by rule. > I don't think this would be a good use of my > time. I would have assumed that the current situation (i.e nitpicking/bikeshedding on the ML) is not a good use of your time :). I would be happy to put some effort to help getting the coding style right, however I believe focusing on an overall coding style would value everyone's time better than a rule by rule discussion. > If I was promised (reasonable) feedback, I could take what I > have and try to add at least a few more things based on what I find > myself commenting on more frequently. But really I'd prefer it to > be done the other way around - for people to look at the patches > already sent, and for me to only subsequently send more. After all, > if already those adjustments are controversial, I don't think we > could settle on others. While I understand this requires another investment from your part, I am afraid it is going to be painful for someone else to go through all the existing coding style bikeshedding and infer your unwritten rules. It might be more beneficial for that person to pursue the work done by Tamas and Viktor in the past (see my previous e-mail). This may mean to adopt an existing coding style (BSD) and then tweak it. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 12:19 ` Julien Grall @ 2020-05-08 12:55 ` Tamas K Lengyel 2020-05-08 14:18 ` Jürgen Groß 0 siblings, 1 reply; 11+ messages in thread From: Tamas K Lengyel @ 2020-05-08 12:55 UTC (permalink / raw) To: Julien Grall Cc: paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: > > Hi Jan, > > On 08/05/2020 12:20, Jan Beulich wrote: > > On 08.05.2020 12:00, Julien Grall wrote: > >> You seem to be the maintainer with the most unwritten rules. Would > >> you mind to have a try at writing a coding style based on it? > > > > On the basis that even small, single aspect patches to CODING_STYLE > > have been ignored [1], > > Your thread is one of the example why I started this thread. Agreeing on > specific rule doesn't work because it either result to bikesheding or > there is not enough interest to review rule by rule. > > > I don't think this would be a good use of my > > time. > > I would have assumed that the current situation (i.e > nitpicking/bikeshedding on the ML) is not a good use of your time :). > > I would be happy to put some effort to help getting the coding style > right, however I believe focusing on an overall coding style would value > everyone's time better than a rule by rule discussion. > > > If I was promised (reasonable) feedback, I could take what I > > have and try to add at least a few more things based on what I find > > myself commenting on more frequently. But really I'd prefer it to > > be done the other way around - for people to look at the patches > > already sent, and for me to only subsequently send more. After all, > > if already those adjustments are controversial, I don't think we > > could settle on others. > While I understand this requires another investment from your part, I am > afraid it is going to be painful for someone else to go through all the > existing coding style bikeshedding and infer your unwritten rules. > > It might be more beneficial for that person to pursue the work done by > Tamas and Viktor in the past (see my previous e-mail). This may mean to > adopt an existing coding style (BSD) and then tweak it. Thanks Julien for restarting this discussion. IMHO agreeing on a set of style rules ahead and then applying universally all at once is not going to be productive since we are so all over the place. Instead, I would recommend we start piece-by-piece. We introduce a baseline style checker, then maintainers can decide when and if they want to move their code-base to be under the automated style checker. That way we have a baseline and each maintainer can decide on their own term when they want to have their files be also style checked and in what form. The upside of this route I think is pretty clear: we can have at least partial automation even while we figure out what to do with some of the more problematic files and quirks that are in our code-base. I would highly prefer this route since I would immediately bring all files I maintain over to the automated checker just so I never ever have to deal with this again manually. What style is in use to me really doesn't matter, BSD was very close with some minor tweaks, or even what we use to check the style just as long as we have _something_. Cheers, Tamas ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 12:55 ` Tamas K Lengyel @ 2020-05-08 14:18 ` Jürgen Groß 2020-05-08 14:23 ` Tamas K Lengyel 0 siblings, 1 reply; 11+ messages in thread From: Jürgen Groß @ 2020-05-08 14:18 UTC (permalink / raw) To: Tamas K Lengyel, Julien Grall Cc: paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On 08.05.20 14:55, Tamas K Lengyel wrote: > On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: >> >> Hi Jan, >> >> On 08/05/2020 12:20, Jan Beulich wrote: >>> On 08.05.2020 12:00, Julien Grall wrote: >>>> You seem to be the maintainer with the most unwritten rules. Would >>>> you mind to have a try at writing a coding style based on it? >>> >>> On the basis that even small, single aspect patches to CODING_STYLE >>> have been ignored [1], >> >> Your thread is one of the example why I started this thread. Agreeing on >> specific rule doesn't work because it either result to bikesheding or >> there is not enough interest to review rule by rule. >> >>> I don't think this would be a good use of my >>> time. >> >> I would have assumed that the current situation (i.e >> nitpicking/bikeshedding on the ML) is not a good use of your time :). >> >> I would be happy to put some effort to help getting the coding style >> right, however I believe focusing on an overall coding style would value >> everyone's time better than a rule by rule discussion. >> >>> If I was promised (reasonable) feedback, I could take what I >>> have and try to add at least a few more things based on what I find >>> myself commenting on more frequently. But really I'd prefer it to >>> be done the other way around - for people to look at the patches >>> already sent, and for me to only subsequently send more. After all, >>> if already those adjustments are controversial, I don't think we >>> could settle on others. >> While I understand this requires another investment from your part, I am >> afraid it is going to be painful for someone else to go through all the >> existing coding style bikeshedding and infer your unwritten rules. >> >> It might be more beneficial for that person to pursue the work done by >> Tamas and Viktor in the past (see my previous e-mail). This may mean to >> adopt an existing coding style (BSD) and then tweak it. > > Thanks Julien for restarting this discussion. IMHO agreeing on a set > of style rules ahead and then applying universally all at once is not > going to be productive since we are so all over the place. Instead, I > would recommend we start piece-by-piece. We introduce a baseline style > checker, then maintainers can decide when and if they want to move > their code-base to be under the automated style checker. That way we > have a baseline and each maintainer can decide on their own term when > they want to have their files be also style checked and in what form. > The upside of this route I think is pretty clear: we can have at least > partial automation even while we figure out what to do with some of > the more problematic files and quirks that are in our code-base. I > would highly prefer this route since I would immediately bring all > files I maintain over to the automated checker just so I never ever > have to deal with this again manually. What style is in use to me > really doesn't matter, BSD was very close with some minor tweaks, or > even what we use to check the style just as long as we have > _something_. Wouldn't it make more sense to have a patch checker instead and accept only patches which change code according to the style guide? This wouldn't require to change complete files at a time. Juergen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 14:18 ` Jürgen Groß @ 2020-05-08 14:23 ` Tamas K Lengyel 2020-05-08 14:42 ` Jürgen Groß 0 siblings, 1 reply; 11+ messages in thread From: Tamas K Lengyel @ 2020-05-08 14:23 UTC (permalink / raw) To: Jürgen Groß Cc: Julien Grall, paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On Fri, May 8, 2020 at 8:18 AM Jürgen Groß <jgross@suse.com> wrote: > > On 08.05.20 14:55, Tamas K Lengyel wrote: > > On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: > >> > >> Hi Jan, > >> > >> On 08/05/2020 12:20, Jan Beulich wrote: > >>> On 08.05.2020 12:00, Julien Grall wrote: > >>>> You seem to be the maintainer with the most unwritten rules. Would > >>>> you mind to have a try at writing a coding style based on it? > >>> > >>> On the basis that even small, single aspect patches to CODING_STYLE > >>> have been ignored [1], > >> > >> Your thread is one of the example why I started this thread. Agreeing on > >> specific rule doesn't work because it either result to bikesheding or > >> there is not enough interest to review rule by rule. > >> > >>> I don't think this would be a good use of my > >>> time. > >> > >> I would have assumed that the current situation (i.e > >> nitpicking/bikeshedding on the ML) is not a good use of your time :). > >> > >> I would be happy to put some effort to help getting the coding style > >> right, however I believe focusing on an overall coding style would value > >> everyone's time better than a rule by rule discussion. > >> > >>> If I was promised (reasonable) feedback, I could take what I > >>> have and try to add at least a few more things based on what I find > >>> myself commenting on more frequently. But really I'd prefer it to > >>> be done the other way around - for people to look at the patches > >>> already sent, and for me to only subsequently send more. After all, > >>> if already those adjustments are controversial, I don't think we > >>> could settle on others. > >> While I understand this requires another investment from your part, I am > >> afraid it is going to be painful for someone else to go through all the > >> existing coding style bikeshedding and infer your unwritten rules. > >> > >> It might be more beneficial for that person to pursue the work done by > >> Tamas and Viktor in the past (see my previous e-mail). This may mean to > >> adopt an existing coding style (BSD) and then tweak it. > > > > Thanks Julien for restarting this discussion. IMHO agreeing on a set > > of style rules ahead and then applying universally all at once is not > > going to be productive since we are so all over the place. Instead, I > > would recommend we start piece-by-piece. We introduce a baseline style > > checker, then maintainers can decide when and if they want to move > > their code-base to be under the automated style checker. That way we > > have a baseline and each maintainer can decide on their own term when > > they want to have their files be also style checked and in what form. > > The upside of this route I think is pretty clear: we can have at least > > partial automation even while we figure out what to do with some of > > the more problematic files and quirks that are in our code-base. I > > would highly prefer this route since I would immediately bring all > > files I maintain over to the automated checker just so I never ever > > have to deal with this again manually. What style is in use to me > > really doesn't matter, BSD was very close with some minor tweaks, or > > even what we use to check the style just as long as we have > > _something_. > > Wouldn't it make more sense to have a patch checker instead and accept > only patches which change code according to the style guide? This > wouldn't require to change complete files at a time. In theory, yes. But in practice this would require that we can agree on a style that applies to all patches that touch any file within Xen. We can't seem to do that because there are too many exceptions and corner-cases and personal-preferences of maintainers that apply only to a subset of the codebase. So AFAICT what you propose doesn't seem to be a viable way to start. Tamas ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 14:23 ` Tamas K Lengyel @ 2020-05-08 14:42 ` Jürgen Groß 2020-05-08 15:50 ` Julien Grall 0 siblings, 1 reply; 11+ messages in thread From: Jürgen Groß @ 2020-05-08 14:42 UTC (permalink / raw) To: Tamas K Lengyel Cc: Julien Grall, paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On 08.05.20 16:23, Tamas K Lengyel wrote: > On Fri, May 8, 2020 at 8:18 AM Jürgen Groß <jgross@suse.com> wrote: >> >> On 08.05.20 14:55, Tamas K Lengyel wrote: >>> On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: >>>> >>>> Hi Jan, >>>> >>>> On 08/05/2020 12:20, Jan Beulich wrote: >>>>> On 08.05.2020 12:00, Julien Grall wrote: >>>>>> You seem to be the maintainer with the most unwritten rules. Would >>>>>> you mind to have a try at writing a coding style based on it? >>>>> >>>>> On the basis that even small, single aspect patches to CODING_STYLE >>>>> have been ignored [1], >>>> >>>> Your thread is one of the example why I started this thread. Agreeing on >>>> specific rule doesn't work because it either result to bikesheding or >>>> there is not enough interest to review rule by rule. >>>> >>>>> I don't think this would be a good use of my >>>>> time. >>>> >>>> I would have assumed that the current situation (i.e >>>> nitpicking/bikeshedding on the ML) is not a good use of your time :). >>>> >>>> I would be happy to put some effort to help getting the coding style >>>> right, however I believe focusing on an overall coding style would value >>>> everyone's time better than a rule by rule discussion. >>>> >>>>> If I was promised (reasonable) feedback, I could take what I >>>>> have and try to add at least a few more things based on what I find >>>>> myself commenting on more frequently. But really I'd prefer it to >>>>> be done the other way around - for people to look at the patches >>>>> already sent, and for me to only subsequently send more. After all, >>>>> if already those adjustments are controversial, I don't think we >>>>> could settle on others. >>>> While I understand this requires another investment from your part, I am >>>> afraid it is going to be painful for someone else to go through all the >>>> existing coding style bikeshedding and infer your unwritten rules. >>>> >>>> It might be more beneficial for that person to pursue the work done by >>>> Tamas and Viktor in the past (see my previous e-mail). This may mean to >>>> adopt an existing coding style (BSD) and then tweak it. >>> >>> Thanks Julien for restarting this discussion. IMHO agreeing on a set >>> of style rules ahead and then applying universally all at once is not >>> going to be productive since we are so all over the place. Instead, I >>> would recommend we start piece-by-piece. We introduce a baseline style >>> checker, then maintainers can decide when and if they want to move >>> their code-base to be under the automated style checker. That way we >>> have a baseline and each maintainer can decide on their own term when >>> they want to have their files be also style checked and in what form. >>> The upside of this route I think is pretty clear: we can have at least >>> partial automation even while we figure out what to do with some of >>> the more problematic files and quirks that are in our code-base. I >>> would highly prefer this route since I would immediately bring all >>> files I maintain over to the automated checker just so I never ever >>> have to deal with this again manually. What style is in use to me >>> really doesn't matter, BSD was very close with some minor tweaks, or >>> even what we use to check the style just as long as we have >>> _something_. >> >> Wouldn't it make more sense to have a patch checker instead and accept >> only patches which change code according to the style guide? This >> wouldn't require to change complete files at a time. > > In theory, yes. But in practice this would require that we can agree > on a style that applies to all patches that touch any file within Xen. > We can't seem to do that because there are too many exceptions and > corner-cases and personal-preferences of maintainers that apply only > to a subset of the codebase. So AFAICT what you propose doesn't seem > to be a viable way to start. I think long ago we already agreed to have a control file which tells a style checker which style to apply (if any). As a start we could have a patch checker checking only the commit message (has it a Signed-off-by: etc.). The next step would be to add the control file, and the framework to split the patch into the changed file hunks and passing each hunk to the correct checking script (might be doing nothing in the beginning). And then we could add some logic to the single checkers. Juergen ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 14:42 ` Jürgen Groß @ 2020-05-08 15:50 ` Julien Grall 2020-05-08 15:56 ` Jürgen Groß 0 siblings, 1 reply; 11+ messages in thread From: Julien Grall @ 2020-05-08 15:50 UTC (permalink / raw) To: Jürgen Groß, Tamas K Lengyel Cc: paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On 08/05/2020 15:42, Jürgen Groß wrote: > On 08.05.20 16:23, Tamas K Lengyel wrote: >> On Fri, May 8, 2020 at 8:18 AM Jürgen Groß <jgross@suse.com> wrote: >>> >>> On 08.05.20 14:55, Tamas K Lengyel wrote: >>>> On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: >>>>> >>>>> Hi Jan, >>>>> >>>>> On 08/05/2020 12:20, Jan Beulich wrote: >>>>>> On 08.05.2020 12:00, Julien Grall wrote: >>>>>>> You seem to be the maintainer with the most unwritten rules. Would >>>>>>> you mind to have a try at writing a coding style based on it? >>>>>> >>>>>> On the basis that even small, single aspect patches to CODING_STYLE >>>>>> have been ignored [1], >>>>> >>>>> Your thread is one of the example why I started this thread. >>>>> Agreeing on >>>>> specific rule doesn't work because it either result to bikesheding or >>>>> there is not enough interest to review rule by rule. >>>>> >>>>>> I don't think this would be a good use of my >>>>>> time. >>>>> >>>>> I would have assumed that the current situation (i.e >>>>> nitpicking/bikeshedding on the ML) is not a good use of your time :). >>>>> >>>>> I would be happy to put some effort to help getting the coding style >>>>> right, however I believe focusing on an overall coding style would >>>>> value >>>>> everyone's time better than a rule by rule discussion. >>>>> >>>>>> If I was promised (reasonable) feedback, I could take what I >>>>>> have and try to add at least a few more things based on what I find >>>>>> myself commenting on more frequently. But really I'd prefer it to >>>>>> be done the other way around - for people to look at the patches >>>>>> already sent, and for me to only subsequently send more. After all, >>>>>> if already those adjustments are controversial, I don't think we >>>>>> could settle on others. >>>>> While I understand this requires another investment from your part, >>>>> I am >>>>> afraid it is going to be painful for someone else to go through all >>>>> the >>>>> existing coding style bikeshedding and infer your unwritten rules. >>>>> >>>>> It might be more beneficial for that person to pursue the work done by >>>>> Tamas and Viktor in the past (see my previous e-mail). This may >>>>> mean to >>>>> adopt an existing coding style (BSD) and then tweak it. >>>> >>>> Thanks Julien for restarting this discussion. IMHO agreeing on a set >>>> of style rules ahead and then applying universally all at once is not >>>> going to be productive since we are so all over the place. Instead, I >>>> would recommend we start piece-by-piece. We introduce a baseline style >>>> checker, then maintainers can decide when and if they want to move >>>> their code-base to be under the automated style checker. That way we >>>> have a baseline and each maintainer can decide on their own term when >>>> they want to have their files be also style checked and in what form. >>>> The upside of this route I think is pretty clear: we can have at least >>>> partial automation even while we figure out what to do with some of >>>> the more problematic files and quirks that are in our code-base. I >>>> would highly prefer this route since I would immediately bring all >>>> files I maintain over to the automated checker just so I never ever >>>> have to deal with this again manually. What style is in use to me >>>> really doesn't matter, BSD was very close with some minor tweaks, or >>>> even what we use to check the style just as long as we have >>>> _something_. >>> >>> Wouldn't it make more sense to have a patch checker instead and accept >>> only patches which change code according to the style guide? This >>> wouldn't require to change complete files at a time. So what are you going to do if a new contributor is unfortunate enough to modify code that doesn't pass the coding style checker? Are we going to impose him/her to fix the coding style before submitting his patch? >> >> In theory, yes. But in practice this would require that we can agree >> on a style that applies to all patches that touch any file within Xen. >> We can't seem to do that because there are too many exceptions and >> corner-cases and personal-preferences of maintainers that apply only >> to a subset of the codebase. So AFAICT what you propose doesn't seem >> to be a viable way to start. > > I think long ago we already agreed to have a control file which tells a > style checker which style to apply (if any). As a start we could have a > patch checker checking only the commit message (has it a Signed-off-by: > etc.). The next step would be to add the control file, and the framework > to split the patch into the changed file hunks and passing each hunk to > the correct checking script (might be doing nothing in the beginning). > And then we could add some logic to the single checkers. Yes the framework can be written down now (patches are welcomed). But this doesn't resolve the underlying problem. Aside files imported from Linux, what is the coding style of Xen? The best way to describe it at the moment is a collection of unwritten rules that can differ even between maintainers of the same component. I don't really see how we could write a style checker based on this. The best way to reduce the burden on reviewer and bikeshedding is by formalizing what our coding style. With that, we can write a style checker effectively and all sort of other tools. It would prefer to have a common one but we could consider small quirks per components if it is necessary. Cheers, -- Julien Grall ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Xen Coding style 2020-05-08 15:50 ` Julien Grall @ 2020-05-08 15:56 ` Jürgen Groß 0 siblings, 0 replies; 11+ messages in thread From: Jürgen Groß @ 2020-05-08 15:56 UTC (permalink / raw) To: Julien Grall, Tamas K Lengyel Cc: paul, Julien Grall, committers, Jan Beulich, xen-devel, Woodhouse, David On 08.05.20 17:50, Julien Grall wrote: > > > On 08/05/2020 15:42, Jürgen Groß wrote: >> On 08.05.20 16:23, Tamas K Lengyel wrote: >>> On Fri, May 8, 2020 at 8:18 AM Jürgen Groß <jgross@suse.com> wrote: >>>> >>>> On 08.05.20 14:55, Tamas K Lengyel wrote: >>>>> On Fri, May 8, 2020 at 6:21 AM Julien Grall <julien@xen.org> wrote: >>>>>> >>>>>> Hi Jan, >>>>>> >>>>>> On 08/05/2020 12:20, Jan Beulich wrote: >>>>>>> On 08.05.2020 12:00, Julien Grall wrote: >>>>>>>> You seem to be the maintainer with the most unwritten rules. Would >>>>>>>> you mind to have a try at writing a coding style based on it? >>>>>>> >>>>>>> On the basis that even small, single aspect patches to CODING_STYLE >>>>>>> have been ignored [1], >>>>>> >>>>>> Your thread is one of the example why I started this thread. >>>>>> Agreeing on >>>>>> specific rule doesn't work because it either result to bikesheding or >>>>>> there is not enough interest to review rule by rule. >>>>>> >>>>>>> I don't think this would be a good use of my >>>>>>> time. >>>>>> >>>>>> I would have assumed that the current situation (i.e >>>>>> nitpicking/bikeshedding on the ML) is not a good use of your time :). >>>>>> >>>>>> I would be happy to put some effort to help getting the coding style >>>>>> right, however I believe focusing on an overall coding style would >>>>>> value >>>>>> everyone's time better than a rule by rule discussion. >>>>>> >>>>>>> If I was promised (reasonable) feedback, I could take what I >>>>>>> have and try to add at least a few more things based on what I find >>>>>>> myself commenting on more frequently. But really I'd prefer it to >>>>>>> be done the other way around - for people to look at the patches >>>>>>> already sent, and for me to only subsequently send more. After all, >>>>>>> if already those adjustments are controversial, I don't think we >>>>>>> could settle on others. >>>>>> While I understand this requires another investment from your >>>>>> part, I am >>>>>> afraid it is going to be painful for someone else to go through >>>>>> all the >>>>>> existing coding style bikeshedding and infer your unwritten rules. >>>>>> >>>>>> It might be more beneficial for that person to pursue the work >>>>>> done by >>>>>> Tamas and Viktor in the past (see my previous e-mail). This may >>>>>> mean to >>>>>> adopt an existing coding style (BSD) and then tweak it. >>>>> >>>>> Thanks Julien for restarting this discussion. IMHO agreeing on a set >>>>> of style rules ahead and then applying universally all at once is not >>>>> going to be productive since we are so all over the place. Instead, I >>>>> would recommend we start piece-by-piece. We introduce a baseline style >>>>> checker, then maintainers can decide when and if they want to move >>>>> their code-base to be under the automated style checker. That way we >>>>> have a baseline and each maintainer can decide on their own term when >>>>> they want to have their files be also style checked and in what form. >>>>> The upside of this route I think is pretty clear: we can have at least >>>>> partial automation even while we figure out what to do with some of >>>>> the more problematic files and quirks that are in our code-base. I >>>>> would highly prefer this route since I would immediately bring all >>>>> files I maintain over to the automated checker just so I never ever >>>>> have to deal with this again manually. What style is in use to me >>>>> really doesn't matter, BSD was very close with some minor tweaks, or >>>>> even what we use to check the style just as long as we have >>>>> _something_. >>>> >>>> Wouldn't it make more sense to have a patch checker instead and accept >>>> only patches which change code according to the style guide? This >>>> wouldn't require to change complete files at a time. > > So what are you going to do if a new contributor is unfortunate enough > to modify code that doesn't pass the coding style checker? Are we going > to impose him/her to fix the coding style before submitting his patch? The modified portions should comply to the coding style. Same as in the Linux kernel. > >>> >>> In theory, yes. But in practice this would require that we can agree >>> on a style that applies to all patches that touch any file within Xen. >>> We can't seem to do that because there are too many exceptions and >>> corner-cases and personal-preferences of maintainers that apply only >>> to a subset of the codebase. So AFAICT what you propose doesn't seem >>> to be a viable way to start. >> >> I think long ago we already agreed to have a control file which tells a >> style checker which style to apply (if any). As a start we could have a >> patch checker checking only the commit message (has it a Signed-off-by: >> etc.). The next step would be to add the control file, and the framework >> to split the patch into the changed file hunks and passing each hunk to >> the correct checking script (might be doing nothing in the beginning). >> And then we could add some logic to the single checkers. > > Yes the framework can be written down now (patches are welcomed). But > this doesn't resolve the underlying problem. Aside files imported from > Linux, what is the coding style of Xen? The checker for Xen style needs to be written, yes. > The best way to describe it at the moment is a collection of unwritten > rules that can differ even between maintainers of the same component. I > don't really see how we could write a style checker based on this. The style checker's rules must be written down, of course. And today there are already some rules documented. > > The best way to reduce the burden on reviewer and bikeshedding is by > formalizing what our coding style. With that, we can write a style > checker effectively and all sort of other tools. Correct. A checker should only check what is written down. Not more, maybe less in the beginning. > It would prefer to have a common one but we could consider small quirks > per components if it is necessary. I would say: only Xen style, Linux kernel style, libxl style (why do we have different styles in one project???), maybe BSD, and maybe no style at all. Ah yes, and Makefile style, doc style, Kconfig style, ... Juergen ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-08 15:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-07 14:43 Xen Coding style Julien Grall 2020-05-08 8:36 ` Jan Beulich 2020-05-08 10:00 ` Julien Grall 2020-05-08 11:20 ` Jan Beulich 2020-05-08 12:19 ` Julien Grall 2020-05-08 12:55 ` Tamas K Lengyel 2020-05-08 14:18 ` Jürgen Groß 2020-05-08 14:23 ` Tamas K Lengyel 2020-05-08 14:42 ` Jürgen Groß 2020-05-08 15:50 ` Julien Grall 2020-05-08 15:56 ` Jürgen Groß
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.