Skip to content

ci: add make_unique rule to check_format#7054

Merged
jmarantz merged 6 commits intoenvoyproxy:masterfrom
lambdai:ciuniq
May 29, 2019
Merged

ci: add make_unique rule to check_format#7054
jmarantz merged 6 commits intoenvoyproxy:masterfrom
lambdai:ciuniq

Conversation

@lambdai
Copy link
Contributor

@lambdai lambdai commented May 23, 2019

Signed-off-by: Yuchen Dai silentdai@gmail.com

As a follow up of #7034

$ ./tools/check_format.py  check tools/testdata/check_format/ |grep cpp_std |grep absl
ERROR: tools/testdata/check_format//cpp_std.cc:7: cpp std: absl::make_unique should be replaced by std::make_unique

tag #7053

@lambdai
Copy link
Contributor Author

lambdai commented May 23, 2019

Nice. It works. Maybe ci rebases from master? I don't see the miss usage at the current commit. Will rebase and fix.

ERROR: From ./source/server/server.cc
ERROR: ./source/server/server.cc:553: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: ./source/server/server.cc:561: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: From ./source/extensions/quic_listeners/quiche/platform/quic_ptr_util_impl.h
ERROR: ./source/extensions/quic_listeners/quiche/platform/quic_ptr_util_impl.h:17: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: From ./source/common/network/addr_family_aware_socket_option_impl.h
ERROR: ./source/common/network/addr_family_aware_socket_option_impl.h:22: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: ./source/common/network/addr_family_aware_socket_option_impl.h:23: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: From ./source/common/router/rds_impl.cc
ERROR: ./source/common/router/rds_impl.cc:219: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: From ./test/common/http/codec_impl_fuzz_test.cc
ERROR: ./test/common/http/codec_impl_fuzz_test.cc:358: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: ./test/common/http/codec_impl_fuzz_test.cc:362: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: ./test/common/http/codec_impl_fuzz_test.cc:369: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: ./test/common/http/codec_impl_fuzz_test.cc:374: cpp std: absl::make_unique should be replaced by std::make_unique
ERROR: check format failed. run 'tools/check_format.py fix'

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for adding the test data. But you'll need to also add some lines to tools/check_format_test_helper.py to run on this file with expected outputs. Scan to the end of that file and you'll see what to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Added

Copy link
Contributor

@jmarantz jmarantz left a comment

Choose a reason for hiding this comment

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

looks great; just a couple of nits.

@lambdai lambdai force-pushed the ciuniq branch 2 times, most recently from c56c2a3 to 4fab27b Compare May 24, 2019 20:25
@jmarantz
Copy link
Contributor

LGTM generally.

Looks like another reference to absl::make_unique got merged into ./test/mocks/upstream/cluster_info.cc; let's fix that one and get this merged.

lambdai added 5 commits May 28, 2019 16:44
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@lambdai
Copy link
Contributor Author

lambdai commented May 28, 2019

Thanks! Looks like this PR has greater value than I expected
Rebase and force pushed

@lambdai
Copy link
Contributor Author

lambdai commented May 28, 2019

oops. looking into the failure

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@@ -0,0 +1,11 @@
#include <memory>

#include "absl/memory/memory.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

btw this file (and the gold) is not actually compiled, so you don't need the includes. You do need the 'namespace Envoy' as the presence of that is one of the formatting rules that is checked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cleaning those out is totally optional IMO.

@jmarantz jmarantz merged commit e25bb86 into envoyproxy:master May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants