Re: Do we want to force braces even for 1 line if/for/etc?
El diumenge, 6 de març de 2022, a les 8:07:08 (CET), Simone Gaiarin va escriure: > Yes, let's do that. Ok, I'll commit on thursday unless somoene strongly disagrees. Cheers, Albert > > > > > On Thu, Mar 3, 2022 at 11:42 PM Albert Astals Cid wrote: > > > El divendres, 6 de novembre de 2020, a les 0:27:05 (CET), Albert Astals > > Cid va escriure: > > > El divendres, 11 de setembre de 2020, a les 22:00:30 CET, Albert Astals > > Cid va escriure: > > > > El divendres, 21 d’agost de 2020, a les 1:19:19 CEST, Albert Astals > > Cid va escriure: > > > > > Most of the guidelines suggest it so that you don't forget to add > > them when adding a new line in the "block". > > > > > > > > > > What do you think? > > > > > > > > > > https://invent.kde.org/graphics/okular/-/merge_requests/248 > > > > > > > > > > We're going to need quite some changes to make it pass, so asking > > before starting to do the work :D > > > > > > > > We agreed on the Akademy Okular meeting that we will do this, *but* > > I'm going to postpone doing it after the 20.08.3 release. > > > > > > > > Rationale: > > > > * Making this change even if mechanical (clang-tidy does it) can > > cause potential regressions if something goes wrong, hence is something > > that needs to be applied in master only > > > > * If we apply it to master now, merging up from release/20.08 to > > master can cause master CI to stop compiling since it'd be requiring more > > things than release/20.08 CI > > > > > > Going to postpone this because it seems not clang-tidy 10 nor 11 nor 12 > > are able to format Okular codebase correctly with > > readability-braces-around-statements > > > > clang-tidy-13 seems to have succeeded. > > > > What do you say, should we give it a go? > > > > https://invent.kde.org/graphics/okular/-/merge_requests/578 > > > > Cheers, > > Albert > > > > > > > > Cheers, > > > Albert > > > > > > > > > > > Cheers, > > > > Albert > > > > > > > > > > > > > > Cheers, > > > > > Albert > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: Do we want to force braces even for 1 line if/for/etc?
Yes, let's do that. On Thu, Mar 3, 2022 at 11:42 PM Albert Astals Cid wrote: > El divendres, 6 de novembre de 2020, a les 0:27:05 (CET), Albert Astals > Cid va escriure: > > El divendres, 11 de setembre de 2020, a les 22:00:30 CET, Albert Astals > Cid va escriure: > > > El divendres, 21 d’agost de 2020, a les 1:19:19 CEST, Albert Astals > Cid va escriure: > > > > Most of the guidelines suggest it so that you don't forget to add > them when adding a new line in the "block". > > > > > > > > What do you think? > > > > > > > > https://invent.kde.org/graphics/okular/-/merge_requests/248 > > > > > > > > We're going to need quite some changes to make it pass, so asking > before starting to do the work :D > > > > > > We agreed on the Akademy Okular meeting that we will do this, *but* > I'm going to postpone doing it after the 20.08.3 release. > > > > > > Rationale: > > > * Making this change even if mechanical (clang-tidy does it) can > cause potential regressions if something goes wrong, hence is something > that needs to be applied in master only > > > * If we apply it to master now, merging up from release/20.08 to > master can cause master CI to stop compiling since it'd be requiring more > things than release/20.08 CI > > > > Going to postpone this because it seems not clang-tidy 10 nor 11 nor 12 > are able to format Okular codebase correctly with > readability-braces-around-statements > > clang-tidy-13 seems to have succeeded. > > What do you say, should we give it a go? > > https://invent.kde.org/graphics/okular/-/merge_requests/578 > > Cheers, > Albert > > > > > Cheers, > > Albert > > > > > > > > Cheers, > > > Albert > > > > > > > > > > > Cheers, > > > > Albert > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
Re: Do we want to force braces even for 1 line if/for/etc?
El divendres, 6 de novembre de 2020, a les 0:27:05 (CET), Albert Astals Cid va escriure: > El divendres, 11 de setembre de 2020, a les 22:00:30 CET, Albert Astals Cid > va escriure: > > El divendres, 21 d’agost de 2020, a les 1:19:19 CEST, Albert Astals Cid va > > escriure: > > > Most of the guidelines suggest it so that you don't forget to add them > > > when adding a new line in the "block". > > > > > > What do you think? > > > > > > https://invent.kde.org/graphics/okular/-/merge_requests/248 > > > > > > We're going to need quite some changes to make it pass, so asking before > > > starting to do the work :D > > > > We agreed on the Akademy Okular meeting that we will do this, *but* I'm > > going to postpone doing it after the 20.08.3 release. > > > > Rationale: > > * Making this change even if mechanical (clang-tidy does it) can cause > > potential regressions if something goes wrong, hence is something that > > needs to be applied in master only > > * If we apply it to master now, merging up from release/20.08 to master > > can cause master CI to stop compiling since it'd be requiring more things > > than release/20.08 CI > > Going to postpone this because it seems not clang-tidy 10 nor 11 nor 12 are > able to format Okular codebase correctly with > readability-braces-around-statements clang-tidy-13 seems to have succeeded. What do you say, should we give it a go? https://invent.kde.org/graphics/okular/-/merge_requests/578 Cheers, Albert > > Cheers, > Albert > > > > > Cheers, > > Albert > > > > > > > > Cheers, > > > Albert > > > > > > > > > > > > > > > > > > > > > > > >
Re: Do we want to force braces even for 1 line if/for/etc?
El divendres, 11 de setembre de 2020, a les 22:00:30 CET, Albert Astals Cid va escriure: > El divendres, 21 d’agost de 2020, a les 1:19:19 CEST, Albert Astals Cid va > escriure: > > Most of the guidelines suggest it so that you don't forget to add them when > > adding a new line in the "block". > > > > What do you think? > > > > https://invent.kde.org/graphics/okular/-/merge_requests/248 > > > > We're going to need quite some changes to make it pass, so asking before > > starting to do the work :D > > We agreed on the Akademy Okular meeting that we will do this, *but* I'm going > to postpone doing it after the 20.08.3 release. > > Rationale: > * Making this change even if mechanical (clang-tidy does it) can cause > potential regressions if something goes wrong, hence is something that needs > to be applied in master only > * If we apply it to master now, merging up from release/20.08 to master can > cause master CI to stop compiling since it'd be requiring more things than > release/20.08 CI Going to postpone this because it seems not clang-tidy 10 nor 11 nor 12 are able to format Okular codebase correctly with readability-braces-around-statements Cheers, Albert > > Cheers, > Albert > > > > > Cheers, > > Albert > > > > > > > > > > >
Re: Do we want to force braces even for 1 line if/for/etc?
El divendres, 21 d’agost de 2020, a les 1:19:19 CEST, Albert Astals Cid va escriure: > Most of the guidelines suggest it so that you don't forget to add them when > adding a new line in the "block". > > What do you think? > > https://invent.kde.org/graphics/okular/-/merge_requests/248 > > We're going to need quite some changes to make it pass, so asking before > starting to do the work :D We agreed on the Akademy Okular meeting that we will do this, *but* I'm going to postpone doing it after the 20.08.3 release. Rationale: * Making this change even if mechanical (clang-tidy does it) can cause potential regressions if something goes wrong, hence is something that needs to be applied in master only * If we apply it to master now, merging up from release/20.08 to master can cause master CI to stop compiling since it'd be requiring more things than release/20.08 CI Cheers, Albert > > Cheers, > Albert > > >
Re: Do we want to force braces even for 1 line if/for/etc?
I am not a fan of these braces---I think they add extra boilerplate lines without making the code easier to read. On 21.08.20 01:19, Albert Astals Cid wrote: > Most of the guidelines suggest it so that you don't forget to add them when > adding a new line in the "block". g++ warns me when I write stuff like if (foo) do_something(); do_something_more(); For me this is good enough. Best, Oliver smime.p7s Description: S/MIME Cryptographic Signature
Do we want to force braces even for 1 line if/for/etc?
Most of the guidelines suggest it so that you don't forget to add them when adding a new line in the "block". What do you think? https://invent.kde.org/graphics/okular/-/merge_requests/248 We're going to need quite some changes to make it pass, so asking before starting to do the work :D Cheers, Albert
