Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
Hello,
I am for it, maybe the next major release will be bumped to 6.0, so going with clang formatting would be good because makes the readability better.
Cheers, Daniel
On 17.05.23 16:01, Victor Seva wrote:
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
--
| ,''`. Victor Seva | | : :' : linuxmaniac@torreviejawireless.org | | `. `' PGP Key ID: 0x51A09B18CF5A5068 | | `- Debian Developer | -----------------------------------------------------------------
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
Daniel-Constantin Mierla writes:
I am for it, maybe the next major release will be bumped to 6.0, so going with clang formatting would be good because makes the readability better.
Which style? https://clang.llvm.org/docs/ClangFormatStyleOptions.html supports many:
LLVM A style complying with the LLVM coding standards
Google A style complying with Google’s C++ style guide
Chromium A style complying with Chromium’s style guide
Mozilla A style complying with Mozilla’s style guide
WebKit A style complying with WebKit’s style guide
Microsoft A style complying with Microsoft’s style guide
GNU A style complying with the GNU coding standards
-- Juha
On 17.05.23 17:22, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
I am for it, maybe the next major release will be bumped to 6.0, so going with clang formatting would be good because makes the readability better.
Which style? https://clang.llvm.org/docs/ClangFormatStyleOptions.html supports many:
LLVM A style complying with the LLVM coding standards
Google A style complying with Google’s C++ style guide
Chromium A style complying with Chromium’s style guide
Mozilla A style complying with Mozilla’s style guide
WebKit A style complying with WebKit’s style guide
Microsoft A style complying with Microsoft’s style guide
GNU A style complying with the GNU coding standards
For quite some time, there is a .clang-format file in the root for kamailio sources folder:
- https://github.com/kamailio/kamailio/blob/master/.clang-format
That's the one used so far for the files formatted in the past, expecting it is the one used by Victor as well.
Cheers, Daniel
On 17/5/23 18:23, Daniel-Constantin Mierla miconda@gmail.com wrote:
For quite some time, there is a .clang-format file in the root for kamailio sources folder:
- https://github.com/kamailio/kamailio/blob/master/.clang-format
That's the one used so far for the files formatted in the past, expecting it is the one used by Victor as well.
Yes, that's used by default
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Cheers,
Henning
-----Original Message----- From: Victor Seva linuxmaniac@torreviejawireless.org Sent: Mittwoch, 17. Mai 2023 16:01 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: [sr-dev] clang-format changes
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
Hi,
On 18/5/23 12:39, Henning Westerholt wrote:
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
Well, that was only me. I discussed enforcing clang-format already in the past [0] and no one was against it. So doing it pretty early in the development process of the new release seemed the best way to finally enforce it.
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Yes, but this kind of change is going to be disruptive no matter when it gets merged. So I apologize if someone had not enough time to send any feedback.
I'm going to send another review with the changes for core. I will wait a bit longer this time.
Cheers
[0] https://kamailio.org/wikidocs/devel/irc-meetings/2022a/
Hello,
why do you complain in this case when you don't even give a chance for others to review your commits pushed to modules you haven't authored, nor you maintain?
- https://github.com/kamailio/kamailio/commit/6183319381573e42b882d05ae1748539...
So others have to do it, only you don't?
Cheers, Daniel
On 18.05.23 12:39, Henning Westerholt wrote:
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Cheers,
Henning
-----Original Message----- From: Victor Seva linuxmaniac@torreviejawireless.org Sent: Mittwoch, 17. Mai 2023 16:01 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: [sr-dev] clang-format changes
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
| ,''`. Victor Seva | | : :' : linuxmaniac@torreviejawireless.org | | `. `' PGP Key ID: 0x51A09B18CF5A5068 | | `- Debian Developer |
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
Hi Daniel,
I just said that I found the merge quite fast, and Victor already replied, thank you for that. I have nothing against the change.
Similar code is used also in other module for the same force send socket functionality. It was also verified in a test setup and a pre-production setup.
But if there are issues in the commit, please let me know, I will fix them (like the clang topic, replied in the other e-mail).
Thank you,
Henning
-----Original Message----- From: Daniel-Constantin Mierla miconda@gmail.com Sent: Donnerstag, 18. Mai 2023 13:29 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org; Henning Westerholt hw@gilawa.com Subject: Re: [sr-dev] Re: clang-format changes
Hello,
why do you complain in this case when you don't even give a chance for others to review your commits pushed to modules you haven't authored, nor you maintain?
- https://github.com/kamailio/kamailio/commit/6183319381573e42b882d05ae1748539...
So others have to do it, only you don't?
Cheers, Daniel
On 18.05.23 12:39, Henning Westerholt wrote:
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Cheers,
Henning
-----Original Message----- From: Victor Seva linuxmaniac@torreviejawireless.org Sent: Mittwoch, 17. Mai 2023 16:01 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: [sr-dev] clang-format changes
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
| ,''`. Victor Seva | | : :' : linuxmaniac@torreviejawireless.org | | `. `' PGP Key ID: 0x51A09B18CF5A5068 | | `- Debian Developer |
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-- Daniel-Constantin Mierla -- www.asipto.com www.twitter.com/miconda -- www.linkedin.com/in/miconda Kamailio World Conference - June 5-7, 2023 - www.kamailioworld.com
Hello,
I didn't complain about your commit, but about your attitude about what you expect from others vs what you do. Victor is also core developer, he could have pushed the commits directly like you did.
So if you want it like that, from now on just do PRs with the commits that affect components and modules that you haven't authored nor maintain and wait to be approved.
Cheers, Daniel
On 18.05.23 13:32, Henning Westerholt wrote:
Hi Daniel,
I just said that I found the merge quite fast, and Victor already replied, thank you for that. I have nothing against the change.
Similar code is used also in other module for the same force send socket functionality. It was also verified in a test setup and a pre-production setup.
But if there are issues in the commit, please let me know, I will fix them (like the clang topic, replied in the other e-mail).
Thank you,
Henning
-----Original Message----- From: Daniel-Constantin Mierla miconda@gmail.com Sent: Donnerstag, 18. Mai 2023 13:29 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org; Henning Westerholt hw@gilawa.com Subject: Re: [sr-dev] Re: clang-format changes
Hello,
why do you complain in this case when you don't even give a chance for others to review your commits pushed to modules you haven't authored, nor you maintain?
- https://github.com/kamailio/kamailio/commit/6183319381573e42b882d05ae1748539...
So others have to do it, only you don't?
Cheers, Daniel
On 18.05.23 12:39, Henning Westerholt wrote:
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Cheers,
Henning
-----Original Message----- From: Victor Seva linuxmaniac@torreviejawireless.org Sent: Mittwoch, 17. Mai 2023 16:01 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: [sr-dev] clang-format changes
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
| ,''`. Victor Seva | | : :' : linuxmaniac@torreviejawireless.org | | `. `' PGP Key ID: 0x51A09B18CF5A5068 | | `- Debian Developer |
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-- Daniel-Constantin Mierla -- www.asipto.com www.twitter.com/miconda -- www.linkedin.com/in/miconda Kamailio World Conference - June 5-7, 2023 - www.kamailioworld.com
Hello Daniel,
just to prevent any misunderstandings - It is perfectly fine with me if Victor pushes directly to git master/maintenance branches etc.. My initial comment was not related to that. I am sorry if this was misunderstood. As said, he is also a core developer.
Best regards,
Henning
-----Original Message----- From: Daniel-Constantin Mierla miconda@gmail.com Sent: Donnerstag, 18. Mai 2023 13:45 To: Henning Westerholt hw@gilawa.com; Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: Re: [sr-dev] Re: clang-format changes
Hello,
I didn't complain about your commit, but about your attitude about what you expect from others vs what you do. Victor is also core developer, he could have pushed the commits directly like you did.
So if you want it like that, from now on just do PRs with the commits that affect components and modules that you haven't authored nor maintain and wait to be approved.
Cheers, Daniel
On 18.05.23 13:32, Henning Westerholt wrote:
Hi Daniel,
I just said that I found the merge quite fast, and Victor already replied, thank you for that. I have nothing against the change.
Similar code is used also in other module for the same force send socket functionality. It was also verified in a test setup and a pre-production setup.
But if there are issues in the commit, please let me know, I will fix them (like the clang topic, replied in the other e-mail).
Thank you,
Henning
-----Original Message----- From: Daniel-Constantin Mierla miconda@gmail.com Sent: Donnerstag, 18. Mai 2023 13:29 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org; Henning Westerholt hw@gilawa.com Subject: Re: [sr-dev] Re: clang-format changes
Hello,
why do you complain in this case when you don't even give a chance for others to review your commits pushed to modules you haven't authored, nor you maintain?
- https://github.com/kamailio/kamailio/commit/6183319381573e42b882d05ae1 748539f7547d8c
So others have to do it, only you don't?
Cheers, Daniel
On 18.05.23 12:39, Henning Westerholt wrote:
Hello,
the PR related to this proposal was merged quite fast, less then 24h after creation. Why this was merged already, given that it affects all modules?
I have nothing against this change, but it would have been polite to give it a bit more time for feedback from other module authors, especially as today is a public holiday in many countries. It might also cause conflicts with other people work in progress code.
Cheers,
Henning
-----Original Message----- From: Victor Seva linuxmaniac@torreviejawireless.org Sent: Mittwoch, 17. Mai 2023 16:01 To: Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org Subject: [sr-dev] clang-format changes
Hi,
I'm thinking we should start enforcing clang-format in order to have a coherent indentation and coding style in the project.
I've added a check in the pull-request Github Action to check the clang-format in the commits but for sure now it will almost always fail due to our code is not always formatted properly.
Is is OK if I start committing changes module by module of just changes related to clang-format formatting?
I'm aware this will our life harder to backport fixes to supported branches but I think it will help us in the long run.
Cheers
| ,''`. Victor Seva | | : :' : linuxmaniac@torreviejawireless.org | | `. `' PGP Key ID: 0x51A09B18CF5A5068 | | `- Debian Developer |
Kamailio (SER) - Development Mailing List To unsubscribe send an email to sr-dev-leave@lists.kamailio.org
-- Daniel-Constantin Mierla -- www.asipto.com www.twitter.com/miconda -- www.linkedin.com/in/miconda Kamailio World Conference - June 5-7, 2023
- www.kamailioworld.com
-- Daniel-Constantin Mierla -- www.asipto.com www.twitter.com/miconda -- www.linkedin.com/in/miconda Kamailio World Conference - June 5-7, 2023 - www.kamailioworld.com