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.

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

Auto generate protobuf C++ files #616

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

lucienlu-aws
Copy link
Contributor

@lucienlu-aws lucienlu-aws commented Dec 13, 2024

Issue #, if available:

Description of changes:
Remove the committed protobuf files in favor of auto generating them during bootstrap

Tested with sample producer and consumer

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

CMakeLists.txt Outdated
target_include_directories(kinesis_producer SYSTEM PUBLIC ${THIRD_PARTY_INCLUDE})
target_link_libraries(kinesis_producer ${CMAKE_THREAD_LIBS_INIT} LibProto ${AWSSDK_LINK_LIBRARIES} ${LIBBACKTRACE_LIBRARIES} ${STATIC_LIBS} ${UUID_LIBRARIES} LibSsl LibCurl)
target_link_libraries(kinesis_producer ${CMAKE_THREAD_LIBS_INIT} LibProto ${AWSSDK_LINK_LIBRARIES} ${LIBBACKTRACE_LIBRARIES} ${STATIC_LIBS} ${UUID_LIBRARIES} LibSsl LibCurl ${Protobuf_LIBRARIES})
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this may already part of LibProto, have you checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the addition and reran bootstrap and sample and it still works, so we can remove this change

@@ -52,12 +57,6 @@ set(SOURCE_FILES
aws/kinesis/core/shard_map.h
aws/kinesis/core/user_record.cc
aws/kinesis/core/user_record.h
aws/kinesis/protobuf/config.pb.cc
aws/kinesis/protobuf/config.pb.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add or update a .gitignore files for these generated files so that it doesnt show up as new files that need to be stashed etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated .gitignore

Copy link
Contributor

@akidambisrinivasan akidambisrinivasan left a comment

Choose a reason for hiding this comment

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

If you have run some tests to make sure this works, this looks good.

@lucienlu-aws lucienlu-aws merged commit 9aaab05 into awslabs:master Dec 13, 2024
@lucienlu-aws lucienlu-aws deleted the cpp-generate-proto branch December 13, 2024 19:38
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