[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Coding Style Review and Automation



On Tue, Feb 11, 2025 at 09:10:38AM +0000, Luca Fancellu wrote:
> Hi Roger,
> 
> >> 
> >> 3) The size of the patch after applying clang-format is huge. Really. 
> >> Something
> >> like 9 MB. Even if everyone agrees that the approach is good and we can 
> >> proceed
> >> with it, it is highly unlikely anyone will be able to review it. 
> >> Considering
> >> that new patches are being added to the upstream during such a review, it 
> >> may
> >> also lead to new code style violations or require a new review of that huge
> >> patch.
> > 
> > I think this approach is difficult.  It would likely introduce a lot
> > of noise when using `git blame` (I know, it's just one extra jump,
> > but...), plus would likely break every patch that we currently have
> > in-flight.
> 
> I think we already discussed this in the past and having some churn was 
> accepted,
> also about breaking existing patches, every change merged has the potential 
> to do
> that, this one is more likely but it’s the game I guess?

Hm, maybe, I don't get rebasing issues very often TBH.  Not sure how
intrusive such patch would be.

> > 
> >> 4) Which clang-format version should we set as the one used by Xen, so it 
> >> is
> >> easy for everyone to use it on their hosts?
> >> 
> >> 5) You name it. I think many people in the community can name their points 
> >> for
> >> and against clang-format.
> > 
> > What are the parts of our coding style that clang-format cannot
> > correctly represent?  Could you make a list of what would need to
> > change in Xen coding style for it to match perfectly what clang-format
> > will check?
> 
> we already went through that route, there is no checker anywhere that matches
> the Xen coding style perfectly, so it’s either we change the coding style or 
> we
> don’t proceed further with any automatic check

I'm probably fine with changing coding style, that's why I'm asking
for a list of what needs to be changed (unless we switch to a
completely different coding style).

> > 
> > Ideally the first step would be to prepare a patch to adjust the
> > coding style so it's in line with what clang-format will do.
> 
> It’s easy to say that, but difficult to implement, if we could accept the 
> clang-format
> rules it would be easier to adopt the configuration itself as coding style, 
> maybe
> enhanced with some comments.

I'm kind of lost, why is it difficult to implement?  What I'm asking
for is a patch to CODING_STYLE that modifies it in a way that we could
use clang-format.  In any case we need to do that if we want to use
clang-format.

One question that seems to have been dropped from my previous email:
would it be feasible to apply the updated style to newly added chunks
of code only, but not to the (unmodified) surrounding context?

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.