Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add clang-format to CI #1080

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add clang-format to CI #1080

wants to merge 5 commits into from

Conversation

andarut
Copy link
Contributor

@andarut andarut commented Aug 27, 2024

No description provided.

@andarut andarut requested a review from a team as a code owner August 27, 2024 23:46
@andarut andarut force-pushed the andreylzmw/clang-format branch 4 times, most recently from cc549fb to 2ed64fa Compare August 28, 2024 00:04
pavelshulgin
pavelshulgin previously approved these changes Aug 28, 2024
View reviewed changes
astrophysik
astrophysik reviewed Aug 28, 2024
View reviewed changes
Copy link
Contributor

@astrophysik astrophysik left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay as for me. But let's wait for approve from @Danil42Russia

apolyakov
apolyakov requested changes Aug 28, 2024
View reviewed changes
runtime-light/stdlib/rpc/rpc-buffer.h
@@ -43,7 +43,7 @@ class RpcBuffer : private vk::not_copyable {
}

void reset(size_t pos) noexcept {
php_assert(pos >= 0 && pos <= size());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think it should be changed this way?

Copy link
Contributor

@astrophysik astrophysik Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I remember clang-format gives or gcc with -Werror an error that size_t can't be less than 0

mkornaukhov03
mkornaukhov03 requested changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@mkornaukhov03 mkornaukhov03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that it now formats with default rules? I think we should firstly decide about rules we need before merging into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astrophysik astrophysik astrophysik left review comments

@mkornaukhov03 mkornaukhov03 mkornaukhov03 requested changes

@apolyakov apolyakov apolyakov requested changes

@pavelshulgin pavelshulgin pavelshulgin left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@andarut @apolyakov @pavelshulgin @astrophysik @mkornaukhov03