On Tue, Feb 11, 2025 at 03:06:08PM +0100, Roger Pau Monné wrote: > On Tue, Feb 11, 2025 at 11:19:23AM +0100, Jan Beulich wrote: > > On 11.02.2025 10:10, Luca Fancellu wrote: > > >>> 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? > > > > That's easy to say if you have just a few patches in flight, yet I'm worried > > about this when considering the hundreds of mine that are awaiting review. > > There are also downstreams (including distros) with varying length of > patch queues on top of Xen. Arguably they have to rebase the queue > every time they update, but a wide change in coding style will likely > be fairly disruptive to them. > > Don't take this as a reason to reject clang-format. As mentioned > elsewhere I think the format supported by clang-format would need to > be fairly similar to the current Xen one (up to the point that chunks > of code using the new and the old style could live together). Then we > would enforce it only for newly added chunks of code initially IMO.
While clang-format surely will force some changes (the earlier 9MB patch is a data point here...), I generally expect it should match fairly close to the current code style, and so the rebase shouldn't be _that_ painful. In some cases git likely will handle large part of the work already. It surely is a cost of introducing such a change, but IMO it's a cost worth paying. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab
signature.asc
Description: PGP signature
